Skip to content

Default cache sync to the shared repo and make filters optional - #129

Merged
ErlisLushtaku merged 3 commits into
mainfrom
feat/cache-sync-defaults
Oct 6, 2026
Merged

ErlisLushtaku merged 3 commits into
mainfrom
feat/cache-sync-defaults

Conversation

@ErlisLushtaku

@ErlisLushtaku ErlisLushtaku commented Sep 26, 2026 •

Copy link
Copy Markdown
Collaborator

Description

Makes judgearena-cache usable without spelling out every part of the cache path.

  • Defaults --hf_repo to the public judge-arena/judge-arena-cache dataset, so anyone can fetch without a token while pushing still needs write access to the judge-arena org.
  • Makes --kind, --task and --model optional filters, the omitted ones are matched from the cache folder paths.
  • Requires --all when no filter is given so a bare command does not sync everything by accident, and rejects --all combined with filters.
  • Updates the README examples.

Tested push and fetch against the shared repo through the default, and an anonymous fetch --all.

Comment thread judgearena/cache/hf.py Outdated
task: str | None,
model_spec: str | None,
) -> bool:
if len(parts) != 5 or parts[0] != kind:

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What is the purpose of the length check?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cache folders are always kind/task/provider/model/descriptor_hash so 5 parts means the path length is correct. I'll make it self-explanatory by unpacking the parts instead of a bare 5.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this is a bit hard coded. I would prefer to write some structure that verifies this easily. Perhaps you can create some class like

class CacheSuffix:
kind : Literal["VLLM","OpenRouter"...]
task: ...
provider : ...
model : ...
...

which may or may not inherit some type of os Path class (not sure). Then we can just use this structure instead of parts. Do you think this is an over-engineering? I would love to hear your opinions about it.

I think the problem is if we want to remove provider, then we need to change this everywhere in the code; like we have to update len(parts) == 4 and so on. This way we can get rid of these hard codings

@ErlisLushtaku ErlisLushtaku Oct 2, 2026 •

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's a good idea. I will try it out to see if it gets complex or actually simplifies things.

@ErlisLushtaku ErlisLushtaku Oct 2, 2026 •

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Went with a frozen CacheFolder dataclass in sqlite.py that both builds and parses the relative path, so the layout is defined once. I didn't subclass Path, since remote HF paths are PurePosixPath and local ones are Path.

Additionally, I made local and remote listing share one filter, and kind as an optional filter same as task/model. See d7a93fd.

Lmk what you think.

Define the kind/task/provider/model/descriptor_hash layout once in a
CacheFolder dataclass that both builds and parses relative paths, so the
HF sync no longer hard-codes path depth or segment positions. Local and
remote listing share one filter, and kind becomes an optional filter like
task and model, which removes the per-kind loop in the CLI.

@kargibora kargibora left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think thats better! LGTM

@ErlisLushtaku
ErlisLushtaku merged commit 2795883 into main Oct 6, 2026
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants