feat: remote catalog refresh with caching (#76) - #231
Open
albin-george-kurian wants to merge 1 commit into
Open
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Closes #76.
The catalog was effectively bundled-only:
RemoteRegistryexisted but was neverconstructed anywhere, and
registry_urlwas parsed, env-mapped and printed bymodeldock config showwhile being read nowhere insrc/. Even when constructedmanually it did not do its job — it never cached, and a successful fetch replaced
the catalog instead of extending it, while
search()andrecommend()delegatedstraight to the bundled fallback, so a model that existed only in the remote catalog
could never be found. That is the exact case the feature exists for.
This rebuilds
RemoteRegistryon the sharedCachedCatalogRegistrypipeline, makesit cache-first with a TTL, merges its entries over the bundled catalog rather than
replacing them, and finally wires
registry_urlintoModelManagerso the settingreaches discovery.
Changes
src/modeldock/adapters/registry/remote.py— rewritten as aCachedCatalogRegistrysubclass. Caches to
<cache_dir>/remote_catalog_cache.jsonwith a 1-hour TTL, readcache-first so a configured URL costs no network round-trip per CLI invocation;
falls back to an expired cache before giving up, and
refresh()bypasses the TTL.Remote entries are merged over bundled (remote wins a name collision, no bundled
model is ever dropped). Adds alias/case-aware lookup, per-entry error tolerance so
one malformed record cannot discard a whole payload, honest
describe()reporting,http(s)-only URL validation, and a size-capped streamed response body.src/modeldock/adapters/registry/bundled.py— extractedcatalog_entry_to_specandload_bundled_catalogso the entry coercion is shared instead of reached for throughBundledRegistry._to_spec.BundledRegistrybehaviour is unchanged.src/modeldock/core/manager.py—catalog_source="remote"selectsRemoteRegistry(raising
ConfigErrorwhen noregistry_urlis set); under"auto"a configured URLmerges in ahead of every other source. An unusable URL degrades to a warning rather
than breaking discovery.
src/modeldock/common/config.py—"remote"added to thecatalog_sourceallow-list.src/modeldock/adapters/registry/composite.py—describe()deduplicates sources byname. Without this,
modeldock sourceswould list "Bundled" twice under"auto",since
RemoteRegistrymerges bundled internally and the base registry may also be it.tests/unit/test_remote_registry.py— new, 35 tests.Architecture.md(§9 registry bullet + config paragraph),Development.md,CHANGELOG.md([Unreleased]), and thecatalog_sourcetables inREADME.md/QUICKSTART.md, which enumerated the valid values and were incomplete withoutremote.Note on one design decision
The
CachedCatalogRegistrybase is network-first, with the cache only as an offlinefallback.
RemoteRegistrydeliberately overrides_load/_remote_entriesto becache-first (fresh cache → network → stale cache → bundled). Network-first would leave
the TTL decorative and keep a round-trip in front of every
search/list— which wasone of the original defects. The departure is documented in the method docstring.
Testing
pytest— 647 passed, 4 skipped (skips pre-existing: Ollama CLI not installed).tests/unit/test_remote_registry.py— 35 unit tests driving a realHTTPServeron areal socket rather than mocking httpx, with a request counter. The counter is the
point: a registry that re-fetches per construction passes a mocked test and still
hits the network on every command. Covers fetch/parse, alias and case resolution,
search/recommend/by_category reaching remote entries, merge with bundled, remote
winning a name collision, cache written, second construction served with zero
further requests, TTL expiry re-fetching, expired-cache fallback when the server is
down, bundled-only when there is neither, malformed entry skipped, error status and
unusable payloads degrading, oversized body refused, non-
http(s)URLs rejected,refresh()bypassing the TTL and surviving failure,describe()accuracy, and theModelManagerwiring for both"remote"and"auto".ruff check src tests— All checks passed.ruff format --check— 116 files alreadyformatted.
mypy --strict src— Success, no issues in 80 source files.bandit -c pyproject.toml -r src— 0 issues (0 high / 0 medium / 0 low).modeldock searchfinds the fresh model stampedRemote registry;modeldock inforesolves it by alias; bundled entries remain present and correctly stamped; a second
command issues no second HTTP request;
modeldock sourcesreports the remote'sown contribution (1) separately from bundled (19);
modeldock sources refreshbypasses the TTL.
Checklist
feature/,fix/,docs/,refactor/,test/,chore/)mainAGENT.mdcoding standards (type hints, Pydantic v2, no genericException, no business logic in CLI)domain/andports/stay pure (no I/O, no framework imports)ruff,mypy --strict,bandit,pytestpyproject.tomlandsrc/modeldock/__init__.pyversions match (if release-related)