Ask CortexDB for every scope instead of its first fifty - #142
Conversation
`CortexDialect::scopes` called `v1/scopes/list` with no parameters. The endpoint caps the listing at fifty and documents neither the cap nor the `limit` that lifts it: its OpenAPI entry lists no parameters at all, and the response carries no cursor, no `has_more` and no total, so there is nothing in it that tells a caller it was cut short. That listing is what `entries` and `namespace_summaries` enumerate, and so what `export_page` walks. A deployment holding more namespaces than the cap exported a subset of itself and reported success — a live instance measured 93. Per-namespace reads address a namespace directly and never notice, which is why this stayed invisible. Ask for a limit far above any namespace count, and refuse a response that fills it rather than returning it: with no cursor, a full page and a truncated one are the same response, and the same reasoning as MAX_PAGES applies — a loud error beats a short answer the caller cannot detect. The test double now models the cap, so every Cortex conformance test exercises the real shape rather than a listing that always returns everything.
How this change flows4 changed behaviours across 9 relationships. 5 surrounding behaviours are shown (60 graph nodes walked). 45 further behaviours left out to keep the diagram readable. flowchart LR
n0["...lay_returns_the_event_its_own_key_created<br/>changed"]:::changed
n1["cortex_recall<br/>changed"]:::changed
n2["cortex_scopes<br/>changed"]:::changed
n3["CortexDialect<br/>changed"]:::changed
n4["route"]:::impacted
n5["Result"]:::impacted
n6["cortex_backend"]:::impacted
n7["iter"]:::impacted
n8["new"]:::impacted
n0 -->|calls| n6
n1 -->|calls| n7
n1 -->|tests| n7
n2 -->|calls| n7
n2 -->|tests| n7
n6 -->|calls| n4
n6 -->|tests| n4
n8 -->|uses| n3
n8 -->|uses| n5
classDef changed fill:#0d4429,stroke:#238636,color:#e6edf3
classDef impacted fill:#161b22,stroke:#6e7681,color:#c9d1d9
classDef flagged fill:#5a1e02,stroke:#d93f0b,color:#ffffff
classDef blocking fill:#67060c,stroke:#f85149,color:#ffffff
Green: changed behaviour. Grey: surrounding behaviour. Arrows name the call, use, implementation, or test relationship. Orange: has findings. Red: has a finding that blocks the merge. |
There was a problem hiding this comment.
tinysweeper found nothing blocking. Approving.
$0.0121 · 67,996 in / 1,713 out · 16,402 cached (24%) · openrouter/openai/text-embedding-3-small, z-ai/glm-5.2, deepseek/deepseek-v4-flash · 443 embedded
critique: $0.0047 · 25,419 in / 868 out · 8,505 cached (33%) · z-ai/glm-5.2, deepseek/deepseek-v4-flash
security: $0.0056 · 23,462 in / 712 out · 7,897 cached (34%) · deepseek/deepseek-v4-flash, z-ai/glm-5.2
tests: $0.0012 · 13,251 in / 59 out · 0 cached (0%) · deepseek/deepseek-v4-flash
description: $0.0005 · 5,864 in / 74 out · 0 cached (0%) · deepseek/deepseek-v4-flash
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Team Run ID: 📒 Files selected for processing (2)
Included review availability: Your plan provides up to 1 included review per hour; 0 remain after this review. 📝 WalkthroughWalkthroughThe Cortex adapter now requests up to 10,000 scopes and rejects full responses as potentially truncated. The Cortex test double applies the endpoint limit. Tests cover listings beyond the default cap and responses that fill the requested limit. ChangesCortex scope listing
Estimated code review effort: 3 (Moderate) | ~20 minutes Merge Risk: ⚪ Minimal · up to Cortex scope listings now request up to 10,000 entries and fail safely when completeness cannot be established, preventing silent omission of namespaces. The change is ready to merge. Suggested reviewers: Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
Warning Your free Security trial is over. An organization admin can upgrade to Advanced for continuous pull request security review or dismiss this notice. Comment |
What changed
CortexDialect::scopesaskedv1/scopes/listfor nothing and took whatever cameback. The endpoint caps that listing at fifty and documents neither the cap
nor the
limitparameter that lifts it — its OpenAPI entry lists no parametersat all, and the response carries no cursor, no
has_more, and no total. Nothingin a capped response distinguishes it from a complete one.
That listing feeds
entries→namespace_summaries→export_page. Adeployment holding more namespaces than the cap exported a subset of itself and
reported success. A live instance was measured holding 93 scopes and listing
50. Per-namespace reads address a namespace directly, so they never notice; the
truncation only shows up in the paths that enumerate, which is why it went
unseen.
The fix:
limit=10_000, far above any plausible namespace count;cursor there is no way to ask for the rest and no way to tell a full page from
a truncated one, so the same reasoning as
MAX_PAGESapplies: a loud errorbeats a short answer the caller cannot detect.
The conformance double now reproduces the cap (defaults to 50, honours
limit),so every Cortex test runs against the engine's real shape instead of a stub that
always returns everything. A double that returns the whole set cannot catch an
adapter that forgot to ask.
Public API / behavior changes
No API change. One behavior change:
scopes— and thereforenamespace_summariesandexport_page— now returns an error instead of asilently short listing when a deployment exceeds
SCOPE_LIST_LIMIT.Tests
Two new tests in
crates/tinymemory-remote/src/conformance_test.rs:every_namespace_is_listed_past_the_engines_undocumented_default— writes 64namespaces and asserts all 64 come back. Verified to reproduce the bug:
reverted to the old call it fails with
left: 50, right: 64.a_scope_listing_that_fills_the_limit_is_refused_rather_than_trusted— alisting of exactly
SCOPE_LIST_LIMITentries must error, not be returned.Validation
Run from the repository root, all green:
cargo fmt --all -- --checkcargo clippy --all-targets --all-features -- -D warningscargo build --all-targets --all-featurescargo test --all-featuresDeliberately untested
The exact value of the engine's undocumented default. The double pins 50 because
that is what a live v0.9.9 instance returns; if the vendor changes it, the guard
still holds — the adapter names its own limit and refuses a full page either way.
Summary by CodeRabbit
Bug Fixes
Tests