feat(stella-context): domain overlap admits again, against a scope the query actually narrowed (#2333) - #2356
Merged
Merged
Conversation
…e query actually narrowed The evidence gate's domain rung was removed because the caller could not make it query-conditional: a session hands recall its whole vocabulary, so "overlaps the query's domains" degenerated to "carries any tag at all". This restores the channel with the narrowing that makes it real. A recall now carries two domain scopes (`RecallScope`), because they answer different questions and cannot share a value without one becoming wrong. The SESSION scope is the vocabulary: it filters out-of-scope nodes and ranks overlap in the RRF, both correct uses of a value that does not vary per turn. The QUERY scope is what this goal selected — the domains owning the workspace files the goal named, derived in `ScopedStore::query` from the anchors `goal_path_anchors` already computes. Only the query scope may admit. Admission requires it to be a non-empty PROPER SUBSET of the session scope (`evidence::scope_is_query_conditional`). That condition is the whole fix: if the two scopes are equal, "shares a domain with the query" and "is in scope at all" are the same predicate — and the second is already the filter every candidate passed to get here, so the rung would admit the entire in-scope corpus. A scope must narrow to discriminate. Narrowing the session scope instead was the tempting one-value version and is strictly worse: it also drives `node_ids_excluded_by_scope`, so a narrower value silently starts SUPPRESSING memories rather than merely declining to admit them. Hence two fields. Witnessed on both sides of the boundary. In the store, one corpus and one prompt with a single variable changed: `a_full_vocabulary_domain_scope_is_not_evidence` (scope = vocabulary → 0 frames) against `a_narrowed_domain_scope_is_evidence` (scope = one domain → exactly that domain's two notes, the other three refused). The narrowed case returns [] with the channel disabled, so it is a true fail→pass witness. In the CLI, six tests pin the derivation itself, including the sweeping goal that reaches every domain and therefore narrows nothing. `recall_scoped` keeps taking a bare session scope and documents that it can never admit on domain — a caller with no per-query scope gets the conservative behavior by construction rather than by remembering to ask for it. Closes #2333 Refs #2289
`RecallScope` is public and `scope_is_query_conditional` is `pub(crate)`, so the link failed the rustdoc gate under `-D warnings`. Name it in backticks and state the rule inline, which is what a reader of the public doc needs anyway.
`RecallScope` pushed `retrieval.rs` to 1519 lines, past the 1500-line ratchet, and the file is not grandfathered — so it gets split, not baselined (AGENTS.md § "God files"). 1444 lines now, with room. The split is along a real seam rather than wherever the line count fell. `retrieval::scope` owns everything about domain scoping: the two-scope type and the two projections the corpus tag map feeds — `overlap_ranking` (which ranks, against the session scope) and `evidence_ids` (which admits, against the query scope). Putting them side by side is the point: they read the same map through different scopes, the asymmetry is the subtlest thing in retrieval, and it has already been got wrong once (#2289). Both are pure functions over already-loaded rows, matching the idiom of the sibling modules `evidence` and `ranking`; the parent keeps the one query that loads the map.
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Sorry @macanderson, you have reached your weekly rate limit of 500000 diff characters.
Please try again later or upgrade to continue using Sourcery
Contributor
|
The latest updates on your projects. Learn more about Vercel for GitHub. 1 Skipped Deployment
|
Contributor
Reviewer's GuideThis PR reintroduces domain-overlap as an admission channel by introducing a two-part recall scope (session vs per-query) and using the query-narrowed scope to derive domain evidence, while refactoring retrieval scoping into a new module, wiring the scope derivation through the CLI, and updating docs and tests accordingly. Sequence diagram for domain-overlap ranking vs admission with RecallScopesequenceDiagram
actor User
participant ScopedStore
participant ContextStore
participant recall_blocking
participant scope
participant evidence
User->>ScopedStore: query()
ScopedStore->>ScopedStore: query_domain_scope(domains, anchors)
ScopedStore->>ScopedStore: RecallScope { session, query }
ScopedStore->>ContextStore: recall_scoped_excluding(query, &RecallScope, &excluded_ids)
ContextStore->>recall_blocking: recall_blocking(conn, q, excluded)
recall_blocking->>scope: overlap_ranking(&metas, &scoped_domains, &query_domains)
scope-->>recall_blocking: domain_ranked
recall_blocking->>evidence: scope_is_query_conditional(&q.query_scope, &q.domains)
alt [scope_is_query_conditional == true]
recall_blocking->>scope: evidence_ids(&metas, &scoped_domains, &q.query_scope)
scope-->>recall_blocking: domain_evidence
else [scope_is_query_conditional == false]
recall_blocking->>recall_blocking: domain_evidence = []
end
recall_blocking->>evidence: admissible_ids(&anchor_ids, &anchor_adjacent, &pass.distinctive_matchers(), &domain_evidence, &semantic_hits)
evidence-->>recall_blocking: admissible
recall_blocking-->>ContextStore: RecallResult
ContextStore-->>ScopedStore: RecallResult
ScopedStore-->>User: ContextUsage
File-Level Changes
Assessment against linked issues
Possibly linked issues
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
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
1 participant
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.
What
Domain overlap becomes an admission channel again — this time against a scope the query itself narrowed, which is the property that was missing when it shipped and the reason it was removed.
A recall now carries two domain scopes (
RecallScope):sessionDomains::names())queryAdmission requires
queryto be a non-empty proper subset ofsession(evidence::scope_is_query_conditional).Why the proper-subset condition is the whole fix
If the two scopes are equal, "shares a domain with the query" and "is in scope at all" are the same predicate — and the second is already the filter every candidate passed to get here. The rung would admit the entire in-scope corpus, which is precisely what it did: the session hands recall its whole vocabulary and nothing narrowed it, so the gate admitted every tagged node on every prompt, and reflection write-back tags every episode whose turn touched files. That was #2289 reopened at full width, and it is why PR #2298 removed the rung outright rather than patching it.
A scope must narrow to discriminate. Narrowing
sessioninstead was the tempting one-value version and is strictly worse:sessionalso drivesnode_ids_excluded_by_scope, so a narrower value silently starts suppressing memories rather than merely declining to admit them. Hence two fields, not one.How
RecallScope(crates/stella-context/src/retrieval/scope.rs) — the two-scope type.recall_scoped_excludingtakes it;recallandrecall_scopedstay as conveniences and document that they can never admit on domain, so a caller with no per-query scope gets the conservative behavior by construction rather than by remembering to ask.ScopedStore::query,crates/stella-cli/src/contextgraph.rs) —query_domain_scopemaps the goal's anchors throughDomains::domains_for_path. Anchors are already workspace-relative paths (goal_path_anchors), which is exactly what that function consumes, so no URI parsing is involved. An anchor in any other spelling matches no prefix and drops out — narrowing the scope less, which can only make the gate more reluctant to admit. That is the safe direction.ScopedStorenow holds theDomainstaxonomy rather than just its names;ContextQuerystays workspace-agnostic, so the wire contract is untouched.Witness
In the store — one corpus, one prompt, a single variable changed:
a_full_vocabulary_domain_scope_is_not_evidenceno_evidence_cut5a_narrowed_domain_scope_is_evidencenote-0,note-3; other three refusedNo note shares a distinctive term with the prompt, so domain overlap is the only channel that can fire — these read the rung directly. The narrowed case is a true fail→pass witness: with the channel disabled it returns
[]against the expected["note-0", "note-3"], verified by neuteringscope_is_query_conditionaland re-running.recall_scoped_alone_never_admits_on_domainpins the convenience API's posture. Four unit tests pin the predicate itself, including that repeats cannot fake a narrowing (set semantics, not list length) and that a scope reaching outside the vocabulary is not a subset.In the CLI — six tests pin the derivation (
query_domain_scope_derivation), including the sweeping goal that names a file in every domain and therefore narrows nothing. That degenerate case is pinned on both sides of the boundary deliberately: the two halves have to agree on it, and it is the exact predicate that shipped inverted.File-size ratchet
RecallScopepushedretrieval.rsto 1519 lines, past the 1500 limit, and the file is not grandfathered — so it was split, not baselined (AGENTS.md § God files). 1444 now.The split follows a real seam rather than the line count:
retrieval::scopeowns domain scoping entire — the type plus the two projections the corpus tag map feeds,overlap_ranking(ranks, againstsession) andevidence_ids(admits, againstquery). Putting them side by side is the point. They read the same map through different scopes, that asymmetry is the subtlest thing in retrieval, and it has already been got wrong once. Both are pure functions over loaded rows, matching theevidenceandrankingsiblings; the parent keeps the query that loads the map.Docs
The three prose sites that describe the channels move together, as they must: the
evidence.rsmodule doc, therequire_evidencedoc comment incrates/stella-cli/src/settings/context.rs, anddocs/spec/adaptive-context/adaptive-context.md§6.1 — which now states the general rule rather than just this instance: admission is strictly narrower than ranking, and a predicate the query does not vary cannot be an admission rung.Verification
cargo test -p stella-context— 175 passed, 0 failed.cargo test -p stella-cli— 1500 unit + 12 integration binaries, 0 failed.cargo clippy -p stella-context -p stella-cli --all-targets -- -D warnings— clean.RUSTDOCFLAGS="-D warnings" cargo doc— clean (exit 0).cargo fmt --all --check— clean.make guards-fast— all guards green,check-file-sizeandcheck-module-reachabilityincluded.Notes on CI
mainis red independently of this branch.shellcheckfails onscripts/test-arena-scripts.sh(arrived via #2328/#2351), which reds every open PR whatever it changed. Filed as #2355 with the fix; deliberately not folded in here, since a shared red gate that several PRs each "helpfully" repair is its own failure mode. This diff touches no shell script.Forward-checked against #2354. That PR (open) tightens
doc-warningsto--document-private-items, which surfaces doc links insidepub(crate)items. Ran this branch under the stricter flag: zero errors in any code this PR adds or touches —scope.rs,evidence.rs,evidence_tests.rs,contextgraph.rs, and every added line ofretrieval.rsare clean. The 9 that do fire are all pre-existing onmainin files this PR does not touch (candidates.rs,ranking.rs,ann.rs,store/domain.rs,store/schema.rs, andretrieval.rs's own module doc), and are exactly what #2354 repairs. So this merges green in either order.Closes #2333
Refs #2289