fix(daemon): mount budget-aware lexical projection build - #552
Conversation
… skips Co-authored-by: Zack Jackson <ScriptedAlchemy@users.noreply.github.com>
…rable Co-authored-by: Zack Jackson <ScriptedAlchemy@users.noreply.github.com>
Co-authored-by: Zack Jackson <ScriptedAlchemy@users.noreply.github.com>
Co-authored-by: Zack Jackson <ScriptedAlchemy@users.noreply.github.com>
Keep 5s/30s as omitted-deadline fallbacks only. Prove BudgetExceeded stays Partial and that a semantic miss does not take down exact, lexical, or graph.
A successfully parsed LCM JSON payload whose object keys carry credential material was collapsed into StructuredSanitizationError::SanitizerUnavailable and then mapped to DetectionError::Receipt, so the sanitizer's own fail-closed quarantine surfaced as 'privacy sanitizer receipt construction failed' in every projection-drain refusal. Give quarantine findings their own typed state (CredentialKeyQuarantine -> StructuredQuarantine), keep SanitizerUnavailable for detector initialization failure (-> Initialization, with InvalidLimits), and reserve Receipt for real canonical/receipt construction faults (new CanonicalEncoding). Co-authored-by: Zack Jackson <ScriptedAlchemy@users.noreply.github.com>
begin_maintenance_drain is only for run_maintenance_checkpoint and crate tests. A public unchecked drain would close admission with no permit. Admission stays closed after Truncate; Database will not call this on the live submit path.
RetrievalBudget.deadline_micros is a request/profile bound, not publication or activation. new_admitted still hard-wires None for the daemon first-query caller.
Codex P2: key quarantine no longer shares the "ambiguous structured document" message. Parse ambiguity stays StructuredQuarantine.
The credential-key quarantine test asserts variant equality directly, which requires PartialEq on the public error enum. Matches the derive set already carried by StructuredSanitizationError. Co-authored-by: Zack Jackson <ScriptedAlchemy@users.noreply.github.com>
…nce-checkpoint-port-2ce6' into codex/pr541-review-fix
A stale snapshot inventory is request-local. Checking blockers after begin_maintenance_drain closed admission and left the writer Draining, so Blocked tore down exact-SQL the same way a foreign permit used to. Leave admission open and the writer Ready.
Prove a longer lane cannot lift a tighter base, and that Some(0) expires immediately instead of falling back to the crate default.
detect_lcm_payload's non-JSON route and sanitize_code_source_bytes collapsed every non-empty quarantine-finding set into DetectionError::StructuredQuarantine, so a parsed TOML/YAML document whose keys carry credential material displayed as parse ambiguity. Route by finding: a malformed-record finding stays the parse-ambiguity quarantine; key-anchored findings from a parsed document surface as CredentialKeyQuarantine, matching the JSON container path. Co-authored-by: Zack Jackson <ScriptedAlchemy@users.noreply.github.com>
… window Co-authored-by: Zack Jackson <ScriptedAlchemy@users.noreply.github.com>
…t-ea75' into codex/pr541-review-fix
…nce-checkpoint-port-2ce6' into codex/pr541-review-fix
…nce-checkpoint-port-2ce6' into codex/pr541-review-fix
Thin wrapper so builder can pass a set request deadline_micros. None still uses the crate 30s fallback. new_admitted stays hard-wired None. No daemon edit.
install_query_owners now builds the lexical projection through new_admitted_with_budget and takes the caller's RetrievalBudget. A set deadline_micros, including Some(0), is used as-is; None is unset and keeps the crate 30s fallback. Both build sites pass the budget actually in reach: persistent graph activation and the test-only memory arm are daemon background passes with no request in scope (the ignored- dependency path activates past the durable commit boundary where requester deadlines stop binding), so they pass the daemon retrieval ceiling, whose unset deadline preserves the crate fallback. Co-authored-by: Zack Jackson <ScriptedAlchemy@users.noreply.github.com>
|
Replace .then_some(Completed).unwrap_or(RetryableFailure) on the mount_current_context_scout_claim_authority result with a plain if/else (clippy obfuscated_if_else fold). No behavior change. Co-authored-by: Zack Jackson <ScriptedAlchemy@users.noreply.github.com>
…t-ea75' into td/pr538-update
ScriptedAlchemy
left a comment
ScriptedAlchemy
left a comment
There was a problem hiding this comment.
td query review of #552 @ bb0154e. Comments only. No daemon edits. No restack.
Constructor mount holds. install_query_owners now calls CodeLexicalProjectionAdapterV1::new_admitted_with_budget(metadata, admitted, build_budget). Query crate untouched. Base is td/query-plan20-override (#545 @ 19e3bd7). Docs match the query helper: set deadline_micros including Some(0) is used as-is; None keeps the crate 30s fallback; lane+base must pass the tighter value.
Some(0) test holds when a budget is passed into install_query_owners: typed BudgetExceeded, nothing memoized; None still builds and memoizes.
Residual (request-path cold build). The two production callers still pass queries::maximum_retrieval_budget(), whose deadline_micros is None:
LatestCompleteCodeIndexV1::production_query_owners(memory / first-query share)- graph activation (
graph_activation.rs)
src/daemon/code_index_scheduler/query_runtime.rs still does latest.production_query_owners()? and then uses request.budget only on retrieve, not on the O(store) lexical build. A request that hits a generation with no memoized owners still gets the 30s crate fallback. Activation using the daemon ceiling is documented and fine. The leftover hole is that request-path cold build: if #545 was "a set request deadline_micros has to be able to win," production_query_owners needs to take/forward that budget (tighter of request + profile). The unit test is the only caller that currently passes a set deadline.
advisory_runtime.rs brace change is unrelated; no query opinion.
Not merging. #545 threads are resolved; live mount stays this PR.
ScriptedAlchemy
left a comment
ScriptedAlchemy
left a comment
There was a problem hiding this comment.
Correction on the request-path residual.
production_query_owners does not cold-build in production. #[cfg(not(test))] fail-closes with "code graph projection has not completed activation" unless owners are already memoized from activation. Phrase/exact then go unavailable. The only live lexical build is persistent activation, and that still passes maximum_retrieval_budget() (deadline_micros: None → crate 30s).
So: constructor mount holds; a set request deadline cannot win because no live caller has a request budget in hand, not because first-query rebuilds with the daemon ceiling. If lexical materialization is activation-owned past the requester deadline boundary, say that as the standing rule and request-win is N/A for this build. If a later first-query cold path is added, it must pass the request budget (tighter-of if lane+base exist), not maximum_retrieval_budget().
The injected Some(0)/None test still holds as constructor proof only.
ScriptedAlchemy
left a comment
ScriptedAlchemy
left a comment
There was a problem hiding this comment.
Confirming the live path.
activate_persistent_graph → install_query_owners(..., &maximum_retrieval_budget()) is the only production lexical build. maximum_retrieval_budget() hard-wires deadline_micros: None, so the crate 30s fallback runs. There is no request in reach on activation; that is correct.
Production first-query is not a build. production_query_owners returns the memoized activation owners, or #[cfg(not(test))] fail-closes with "code graph projection has not completed activation". Phrase/exact then go unavailable. The #[cfg(test)] memory arm can build, and that is also the daemon ceiling (None).
So a set request deadline_micros never wins live. Request-win is N/A for this build unless a later cold path is added that has a request budget in hand (then pass tighter-of lane+base, not maximum_retrieval_budget()). Constructor mount holds. Comments only; no daemon edits.
#545 dropped the stale “unmounted until builder mounts” note on new_admitted (9689e95). Live mount is this PR.
…udget-backoff-9603' into td/pr538-update
…idence-budget-backoff-9603
ScriptedAlchemy
left a comment
ScriptedAlchemy
left a comment
There was a problem hiding this comment.
td query cross-review @ bb0154e. Comments only. No daemon edits. No flatten. No CI retrigger.
Not blocking ready (query lock).
- Mount is
install_query_owners→new_admitted_with_budget. Constructor contract holds (Some(0)set /None→ 30s) on the injected test. - Production first-query does not build:
production_query_ownersshares activation owners or fail-closes. The only live build isactivate_persistent_graph→maximum_retrieval_budget()(deadline_micros: None→ crate 30s). No request in reach; request-win is N/A for this build.
Leftover / not query-owned.
- Still draft. Base is now
td/usecases-cheaper-drain-frontier(query does not retarget). - Unrelated
advisory_runtimeclippy fold. - Cancelled/skipped CI is not a crate fail.
Ready watch is empty until Zack names one.
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: bb0154ee58
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| // request budget is in reach here, so the daemon retrieval | ||
| // ceiling applies: its unset `deadline_micros` truthfully keeps | ||
| // the crate build fallback instead of fabricating a deadline. | ||
| &super::queries::maximum_retrieval_budget(), |
There was a problem hiding this comment.
The only non-test caller passes maximum_retrieval_budget(), which constructs a budget with deadline_micros: None; therefore new_admitted_with_budget always uses the same 30-second fallback as the replaced new_admitted, while only the new unit test exercises a set deadline. Production activation consequently has no real budget-aware path for a tighter lexical-build ceiling; either thread an actual activation budget here or avoid mounting the budget-specific API until one exists.
AGENTS.md reference: AGENTS.md:L75-L77
Useful? React with 👍 / 👎.
…t-ea75' into cursor/mount-lexical-budget-725a Co-authored-by: Zack Jackson <ScriptedAlchemy@users.noreply.github.com>
b507b5c to
ae73d9a
Compare
…t-ea75' into cursor/mount-lexical-budget-725a Co-authored-by: Zack Jackson <ScriptedAlchemy@users.noreply.github.com>
d657e02
into
cursor/lcm-quarantine-not-receipt-ea75
2 participants
Summary
td/query-plan20-override). Root cratetracedecayonly; nocrates/*edits.install_query_ownersinsrc/daemon/code_index_scheduler.rsnow builds the lexical projection throughCodeLexicalProjectionAdapterV1::new_admitted_with_budgetand takes the caller'sRetrievalBudget.new_admittedis gone from the daemon path.deadline_micros, includingSome(0), is used as-is;Noneis unset and keeps the crate 30s fallback. A caller holding both a lane and a base budget must pass the tighter of the two — the existinggraph_budget_for_requestpolicy stays the one tighter-of authority; no second policy was added.What budget is actually in reach (investigated, not prescribed)
The lexical build happens exactly once per generation, only at these sites:
activate_persistent_graph(graph_activation.rs), reached from the background reconcile worker and from the ignored-dependency admission flow. Neither carries a request budget: the worker has no request at all, and the ignored-dependency path activates past the durable commit boundary, where requester deadlines deliberately stop binding (run_ignored_dependency_admissionpasses a fresh non-cancelled flag for exactly this reason). Threading a requester deadline there could strand an already-committed generation unactivated on one caller's budget.production_query_owners. In production that arm fails closed (typedContracterror) and never builds, so no request budget reaches it either.Both sites therefore pass the daemon retrieval ceiling (
queries::maximum_retrieval_budget()), the one budget authority in reach; its unsetdeadline_microstruthfully preserves the crate fallback rather than fabricating a deadline. The tighter-of-lane+base rule applies at the mount's contract and is exercised where lane+base genuinely co-exist (the graph lane viagraph_budget_for_request, pinned on the base branch).Test plan
query_owner_build_honors_set_budget_and_keeps_crate_fallback_when_unset—Some(0)expires immediately as typedRetrievalPortError::BudgetExceededwith nothing memoized;Nonekeeps the crate fallback, builds, and memoizes the one shared owner set.production_query_owners_bind_exact_lexical_and_graph_lanes— unchanged lane binding through the new mount.activation_tests— activation still warms owners once, first query re-decodes nothing.Isolated fixtures only (TempDir git repos + scoped store roots); no live
~/.tracedecay. Draft until isolated tests are green in CI. Do notgh stack link(handled after rebase onto current stack-543 top). No base changes to #509/#538/master/#421.