fix(query): let request budgets override crate deadline fallbacks - #545
Merged
ScriptedAlchemy merged 48 commits intoAug 20, 2026
Merged
Conversation
work_manifest and the executable-binding builder return CatalogValidationError but built operation, codec, binding, capability, and use-case identities from caller-supplied operation names with expect. A non-canonical name now surfaces as a typed InvalidValue instead of a panic inside a Result-returning path.
generate_node_id hand-rolled a nibble table for the digest-to-text encoding that canonical_text::encode_lowercase_hex already owns; the output is byte-identical.
The store canonical-projection test includes exactly one provider-normalization fixture; the other 36 arrive with their consumers on the stacked delivery branch. Also note why the tools-call dispatch arm alone is boxed.
The cfg(test) execution/idle/transaction limits (250ms/250ms/500ms) were tuned for fast local disks; a single near-cap ~4MiB replay-page insert plus fsync exceeds them on hosted CI runners, failing the graph-publication near-limit test on every platform. Raise the test-mode budgets to 1s/1s/2s and derive the lease-expiry and revalidated-batch test timings from the constants instead of literals that straddled the old values, so the expiry proofs hold at any calibration. Production limits are unchanged.
Co-authored-by: Zack Jackson <ScriptedAlchemy@users.noreply.github.com>
ensure_root accepted any pre-existing directory after checking only that it was a real directory, so a group/world-writable or foreign-owned spool root let another local account replace records, metadata, or lease files despite their per-file modes. Existing roots now validate through tracedecay-private-fs's directory authority and creation goes through the same authority, giving Windows spool roots a private ACL for the first time. The spool test fixture creates its roots privately, and a new test proves a group-writable root is refused outright.
Contended try_lock_exclusive surfaces ERROR_LOCK_VIOLATION on Windows, which std does not map to WouldBlock, so cross-process contention was mistyped as Io instead of Busy. Compare against fs2's canonical contended error so the typed state holds on every host.
Hosted Windows runners exposed four tests asserting Unix-shaped file semantics: the private-fs replacement test hand-rolled a legacy MoveFileExW replace that Windows denies while a delete-sharing reader is open (production replaces through std::fs::rename's POSIX-semantics path); the pinned-file block test asserted an ErrorKind whose mapping for ERROR_SHARING_VIOLATION varies by std release instead of the raw contract; and the backup-displacement and a-b-a swap tests simulated external renames the Windows pin denies by design, so they now split into per-platform contracts (Unix proves detection, Windows proves the block).
The sibling spool lease and delivery spool already classify contention through std's TryLockError, which types WouldBlock correctly on every host; the ledger now uses the same idiom instead of an fs2 raw-error comparison, removing the crate's last fs2 dependency.
try_acquire_sidecar_lock only treated WouldBlock as contention, but Windows reports ERROR_LOCK_VIOLATION, so a contended sidecar surfaced as an error instead of a clean skip; classify through the crate's is_lock_contended predicate.
The pinned toolchain's if_same_then_else gate rejects the two identical parent.parent() arms that arrived with the consolidation authority merge; fold the conditions into one nested-store predicate.
lifecycle_lease carried a private byte-identical copy of the is_lock_contended predicate that owner_io already exports through crate::db; the canonical copy keeps the LockFileEx classification comment and every lock site now classifies through it.
The application and rusqlite-runtime test trees each carried three private copies of fixture_abs_root; both now import one shared copy (the existing application tests/common and a new rusqlite tests/common module). Cross-crate copies remain deliberate — a workspace test-util crate for eight lines is not worth an architecture edge.
Six near-identical InvalidValue struct literals in the work catalog collapse into an invalid_identity constructor.
Root validation collapsed every failure into UnsafePath, condemning a path for a transient metadata read error; only privacy and kind violations are UnsafePath now, everything else stays Io.
The exact-root precedence fast path let a populated selected store win even when a candidate manifest also names this exact checkout, so the doctor and status journeys resolved silently instead of surfacing the identity cutover conflict their tests demand (status then died on a debug assertion rendering a store with nodes but no files). Selection flags now travel as a typed StoreSelectionEvidence record, and the fast path requires that no candidate names the exact root; a genuine split identity always reaches the cutover conflict.
The consolidation-authority merge carried a single-pattern match, a manual let-else, duplicate match arms, and a mid-function import that the pinned toolchain's pedantic gate rejects.
Two more envelope fixtures carried bare /tmp roots that fail Path::is_absolute on Windows; both now build through the shared platform-absolute helper.
A bare PATH lookup reaches the rustup shim, which can start a
toolchain re-sync mid-test on hosted runners and fail the cargo
metadata call underneath both architecture probes; env!("CARGO") pins
the exact binary that built the test.
The retired-cleanup materialization writes several near-cap pages in one transaction and still outran the 2s test budget on hosted Linux disks after the appends were fixed; double the test-mode limits to 2s/2s/4s, which the lease proofs scale with automatically.
If cancellation regressed, execute would block on the gate-parked executor until the harness timeout; a watchdog releases the gate after a generous bound so the regression surfaces as the existing assertion failures.
The split-identity guard voided exact-root precedence for any candidate naming the checkout, which broke recovery from unreadable duplicate manifests: those must stay untouched history while the healthy selected store serves. Only a candidate with bounded population evidence now voids the fast path and surfaces the cutover conflict.
The bounded-execution proof asserted a literal two-second ceiling that the widened test budgets now touch; derive the bound from the limit like the other lease proofs.
env!("CARGO") baked the build machine's absolute cargo path into the
nextest archives, which run on different Windows shard machines where
that path does not exist. Prefer the runtime CARGO variable (set when
running under cargo, immune to rustup shim re-syncs) and fall back to
the image's PATH cargo for archive runners.
The architecture tests exec cargo metadata at runtime; shard runners only extracted nextest archives and had no installed toolchain, so the rustup shim started a mid-test component download that flakes with partial-file rename errors. Install the toolchain up front like the build job does.
Installing stable was not enough: the rust-toolchain.toml pin still resolved through the shim, and the two architecture tests run concurrently under nextest, so both rustup processes raced to self-install the pin and corrupted each other's partial downloads. Install the pinned toolchain serially before the tests run.
Exact-flat scans and first-query lexical projection stays O(store); a missing caller deadline no longer leaves those paths unbounded.
Score every visited row, but clone candidate/evidence only for the cap that the heap keeps. Isolated test: 4 examined, 2 materialized.
fix(query): cheaper exact-flat retain + deadline backstop
Preserve the redesign's modular storage suite and provider fixtures while integrating the foundation's cross-platform lock, path, and CI fixes.
…iptedAlchemy/tracedecay into codex/tracedecay-total-redesign-plan
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.
|
ScriptedAlchemy
changed the base branch from
codex/tracedecay-total-redesign-plan
to
td/usecases-cheaper-drain-frontier
August 20, 2026 00:02
![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: 95840eb072
ℹ️ 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".
ScriptedAlchemy
commented
Aug 20, 2026
ScriptedAlchemy
left a comment
ScriptedAlchemy
left a comment
Owner
Author
There was a problem hiding this comment.
Cross-review from tracedecay-semantic (no crate edits, no merge).
Crate-only and the composition lock look right: BudgetExceeded stays Partial, exact/lexical/graph stay Complete. That is not a Plan 20 seat and not a cheaper-green census.
Two issues to fix before this is a serving-path follow-up after #539:
- The Plan 20 naming. Plan 20 is publication ≠ activation. A
RetrievalBudget.deadline_microsoverride is not a Plan 20 override and must not be framed as seating/activating semantic. - The new
*_with_budgetconstructors are unused on the live path. Daemoncode_index_schedulerstill callsnew_admitted(...)(deadlineNone→ 30s crate fallback), so a set request budget does not actually win on first-query lexical build.
Do not treat this deadline override as a seat, and do not flatten the stack.
RetrievalBudget.deadline_micros is a request/profile bound, not publication or activation. new_admitted still hard-wires None for the daemon first-query caller.
ScriptedAlchemy
commented
Aug 20, 2026
ScriptedAlchemy
left a comment
ScriptedAlchemy
left a comment
Owner
Author
There was a problem hiding this comment.
Deep review of #545 @ b53812c (not #548). GitHub comments only. No query edits, no restack, no base change.
Holds. Plan 20 wording is gone. Composition lock is right: BudgetExceeded stays Partial, exact/lexical/graph stay Complete. Crate-only. new_admitted still hard-wires None (30s fallback); daemon mount is builder, not this crate.
Residuals (not a serving bug):
- “Request/profile wins” is sloppy next to
effective_deadline_micros(already on 421): when both lane and base deadlines are set it islane.min(base), not last-writer-wins. This PR’s new semantic test only setsrequest.budget(base staysNone). Don’t let the body imply a longer request can lift a tighter profile. Some(0)is a set deadline (immediate expire), not “use fallback”. Worth an explicit test so 0 cannot silently mean unset.request_deadline_overrides_crate_exact_flat_defaultsecond assert is only!BudgetExceeded— Cancelled/Unavailable would also pass. Pin the expected outcome.request_deadline_overrides_crate_lexical_fallbackonly unit-testsunwrap_or. It never constructs vianew_with_budget/new_inner.
new_*_with_budget as a dead switch-in is fine until builder mounts it. Ready watch stays #538. Left #509.
Prove a longer lane cannot lift a tighter base, and that Some(0) expires immediately instead of falling back to the crate default.
ScriptedAlchemy
commented
Aug 20, 2026
ScriptedAlchemy
left a comment
ScriptedAlchemy
left a comment
Owner
Author
There was a problem hiding this comment.
Re-check @ 7b59acd (residuals only). No restack, no query edits.
Addressed. Tighter-of-two is proven through retrieve_semantic (lane.min(base)): a 2× crate-default lane does not lift a 1µs base. Some(0) with elapsed 0 is BudgetExceeded (elapsed >= deadline), not the crate fallback. The longer-lane path now pins Complete, not !BudgetExceeded. Crate “wins” helper/docs are gone; new_*_with_budget is gone too.
Holds. BudgetExceeded stays Partial. Semantic miss does not take down exact/lexical/graph. new_admitted still hard-wires the 30s crate fallback (daemon mount stays builder).
Cheap leftover (body only, not a serving bug): the PR body still says a set deadline “wins on the budget-aware constructors” and names new_admitted_with_budget plus request_deadline_overrides_crate_lexical_fallback, all removed in af33753. Code is fine.
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.
4 tasks
Live daemon mount is new_admitted_with_budget. new_admitted still hard-wires None.
…er' into td/pr545-update
…er' into td/pr545-update
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
2 participants
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
lane.min(base)).Some(0)is a set deadline (immediate expire), not omitted.new_admittedstill hard-wires the crate 30s fallback. Live daemon mount isnew_admitted_with_budgeton fix(daemon): mount budget-aware lexical projection build #552. A setbudget.deadline_micros, includingSome(0), is used as-is;Noneuses 30s. Callers with lane+base must pass the tighter value.BudgetExceededstays Partial, never a complete empty success. Semantic miss does not take down exact/lexical/graph.Test plan
request_deadline_overrides_crate_exact_flat_defaultset_deadline_zero_is_immediate_expire_not_crate_fallbacktighter_of_lane_and_base_deadline_is_usedrequest_deadline_overrides_crate_lexical_fallbacknew_admitted_with_budget_zero_deadline_is_immediate_budget_exceededDoes not reopen #532. Does not merge #421 or #509. Do not flatten. Do not merge until CI is green. This crate does not edit the daemon.