chore: integration batch E (post-fold commits of #1804, #1818, #1821, #1825) - #1862
Merged
Merged
Conversation
A seal or clone backfill outlives the 30s freshness window. Expiry and a predecessor witness used to clear the newer generation and reseal it. Unchanged sealed bytes now rebind that proof instead. Co-authored-by: Zack Jackson <ScriptedAlchemy@users.noreply.github.com>
The seat-swap witness does not own the lexical full-copy. That copy stays on the retained successor driver. Co-authored-by: Zack Jackson <ScriptedAlchemy@users.noreply.github.com>
Dropping the witness generation-id guard let the graph-on retained reconcile return `Noop` whenever the sealed file digests still matched, including after a commit or branch switch that touches no indexed byte (an empty or docs-only commit). `accept_unchanged_sealed_snapshot` then persisted the new git-metadata sample onto the retained generation's witness, so the stale `reference`/`source_revision` attribution stayed masked until code bytes moved. `finish_retained_reconcile` rebuilds on exactly that drift, and `branch_generations` resolves generations by the commit they sealed, so the retained generation must not outlive it. Gate the accept on the attribution a fresh capture would seal: HEAD's ref must still match the snapshot's, and a snapshot that sealed a revision must still name HEAD's commit. `self.identity` is re-resolved a few lines above, so this adds no walk. A snapshot sealed from a dirty tree carries no revision and keeps the fast path. Verified by `a_moved_commit_refuses_the_sealed_generation_despite_identical_bytes`, which fails on the parent commit with `Some(Noop(..))`. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
The advance that installs exact and lexical owners also copied the prior lexical artifact into the clone successor. That copy ran under reconcile_in_progress, so status stayed non-current for the copy. Leave the successor pending; the retained driver starts it after the seat. Co-authored-by: Zack Jackson <ScriptedAlchemy@users.noreply.github.com>
A worker continuation occupied the pending slot with a wall-clock arrival. The follow-up pass then published an event-to-ready receipt, and a suppressed freshness probe that raced the stamp was charged with it. Continuations stay visible to freshness but are not external arrivals. Co-authored-by: Zack Jackson <ScriptedAlchemy@users.noreply.github.com> (cherry picked from commit aecc70b)
Takes 76c58af: the publication advance that installs exact and lexical owners no longer starts the clone-successor full copy inline. That copy ran under reconcile_in_progress, so freshness stayed non-current for its duration. The successor is left pending for the retained driver. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Takes c6aadcb, 08c3f3c and c1633dd: an expired 30s admission clock or a predecessor disk witness no longer clears a sealed generation, because the sealed file digests are the proof. c1633dd then gates that accept on HEAD's ref and commit still matching the snapshot, so a moved commit with identical bytes still refuses and rebuilds. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Exclusive store locks blocked in File::lock, which cannot observe the caller's cancel flag or carried deadline, so a cancelled retention pass waited for a peer's critical section. Acquisition now polls try_lock until the deadline and answers GenerationStoreBusy or Cancelled instead. A free lock is still taken even past the deadline, so an uncontended caller is never refused. The text-artifact candidate scan also treats a vanished file as absent: verify_unreferenced_completed_text_artifact returns whether the artifact was there, the census skips a candidate that is already gone, and the two sites that verify a descriptor the durable index names turn absence into UnsafeState rather than silence. Conflict resolution, crates/tracedecay-code-index-retention/src/ code_index_generations.rs and code_index_generations/generation_scan.rs: resolved to batch C/D's landed design and dropped this commit's competing one. Both fix the same defect, a path unlinked between readdir and open. Batch C/D maps that NotFound to GenerationStoreBusy via deferred_if_absent (690e843, narrowed by d6d8665) and defers the pass; this commit returned Option and skipped the entry. Those two call sites sit in sweep_unreferenced_generation_segments, the loop that builds live_segments, and the code below it removes every segment that set does not contain. Skipping a vanished manifest there under-counts the live set and deletes segments a live generation still references, so deferring is the answer that cannot lose data. Fix Root Causes decided it: the deferral addresses the same race without trading a loud failure for a silent deletion. Its test, 6eecf5f ("assert a vanished census open is absent"), asserts the dropped design and is not folded. Batch C/D's deferral already satisfies that test's stated contract, that a vanished census open is not CodeGenerationRetentionErrorV1::Storage. The text_artifacts.rs staging-sidecar hunk from this commit was already present verbatim in batch C/D and merged as identical content. (cherry picked from commit 7c22827) Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Dropping the 64-attempt busy loop stopped a cancelled caller waiting several seconds, but `ExactSqlError::Busy` answers two questions with one variant: `map_writer_send_error` returns it when the exact-SQL command queue is full, and `begin_immediate` returns it when the writer's own 64ms `EXACT_SQL_WRITE_LOCK_ACQUIRE_LIMIT` loop is exhausted. Treating the first as a final lock answer turned ordinary queue backpressure into `Infrastructure`, which publication reads as `unavailable`. Bound the acquisition by one 64ms wall clock instead of an attempt count. An exhausted lock attempt has already spent that window inside `begin_immediate`, so it still gets exactly one attempt and is never multiplied; a queue refusal returns at once and retries while the window lasts, re-checking interruption each pass. `begin_read` uses the same budget, restoring several context-checked 10ms reader slices before the deferred fallback, and now checks interruption immediately before that fallback, which waits the writer without consulting the context. Resolves the Codex review findings on PR #1821 (P2 admission busy, P1 reader cancellability). Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> (cherry picked from commit 014ba1d)
`checked_acquire_returns_busy_when_the_carried_deadline_has_elapsed` asserts the exact `(1, 0)` acquire shape after resetting a counter, but GRAPH_REPLAY_POOL_ACQUIRE_TRIES/WAITS were process-wide statics. The harness runs the other pool-acquire tests in parallel on their own threads, so any acquire landing between this test's reset and its read inflated the proof: it failed 2 of 25 `-p tracedecay-code-index-retention --lib` runs with `left: (5, 3)`. An acquire runs on its caller's thread, so the observation belongs there. Move both counters to `thread_local!` cells. The assertion is unchanged and now proves only the acquire the test performed; 30 consecutive suite runs are clean. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> (cherry picked from commit 1dca5ba)
preparation_defers_when_the_scope_root_does_not_exist_yet asserted GenerationStoreBusy for a store directory that does not exist. 690e843 made that deferral to stop a missing path being Storage(NotFound). 218ae5c then chose a different answer for this one input: a store directory that has not been created is an unpublished plan, because a waiter can plan against latest_generation_id before cold open creates the scoped store. It did not update this test, so the assertion has been stale since. The expectation now names the contract 218ae5c established, and expect() still fails the test if the call returns any error, which is the defect 690e843 fixed. It also pins the plan shape a waiter reads: no active generation, no pointer, nothing collectable, and the caller's readable sources carried through. Verified failing on origin/ci/pr-batch-d with the crate otherwise unmodified, so this is not a regression from the four folded deltas. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Findings from the deslop and no-comments pass over this batch's added lines. Every change is to a line this batch introduced. locking.rs carried a wrapper pair and a one-valued parameter that nothing used. acquire_scope_retention_lock_checked had exactly one caller, the five-line wrapper directly above it, so the two collapse into one function. lock_file_checked had no unchecked sibling to distinguish it from, and its `busy` parameter was GenerationStoreBusy at both call sites. Only acquire_code_generation_store_lock_checked keeps its deadline and cancel parameters, because the pool-lock tests call it with both to prove the busy and cancelled answers. It drops to pub(super): mod locking is private and the crate re-export never named it. registry.rs: note_worker_continuation's doc said stamping a wall-clock instant was the defect, but the function still stamps one and must, or has_pending_arrival stops seeing the continuation. The defect was publishing that stamp as attributable, so the doc now names attributable = false. graph_publication/support.rs: acquire_within_begin_budget returns Ok(None) for a non-Busy error as well as for an expired budget, which its doc did not say. The Busy arm also re-implemented ensure_not_interrupted, which the same loop already calls four lines above. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Takes batch D's resolution of the missing-scope-root contract: the root-case test is deleted there (735bfbd) and #1836's own test already pins the unpublished plan, so the rewrite from db2a28a is dropped. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> # Conflicts: # crates/tracedecay-code-index-retention/src/code_index_generations/tests.rs
|
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
![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: 67fb6445f4
ℹ️ 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".
Comment on lines
+2074
to
+2075
| _ => match Self::stated_type_path(state, child) { | ||
| Some(path) => clause.paths.push(path), |
There was a problem hiding this comment.
When a generic function is inside an inline module and its bound names a module-local trait, this stores only the written bare path (for example, Processor) and later emits Processor::process. The same-file resolver indexes the actual method as inner::Processor::process; consequently the call is either left unresolved or, if a top-level trait with the same name exists, incorrectly bound to that top-level method. Qualify local bound paths with the enclosing module scope, while retaining the unqualified/import-resolved alternative where needed.
Useful? React with 👍 / 👎.
This was referenced Sep 19, 2026
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.
Integration branch on top of #1860 (batch D). It folds the commits four PRs received after #1848 had folded their earlier heads, so those PRs close as merged once this lands.
Folded:
reconcile_in_progress; a sealed generation is kept when its proof expires, refused when the checkout moved past it).try_lockto the carried deadline and answerGenerationStoreBusyorCancelledinstead of blocking forever; the graph-publication begin is bounded by a 64 ms wall clock rather than one attempt; two acquire counters become thread-local so parallel tests cannot inflate each other's assertions. Its fourth commit is not folded: it skipped a vanished manifest inside the segment sweep, which would delete segments a live generation still references. The batch already defers that race.Also drops an unused checked-lock wrapper pair and a one-valued parameter, and takes batch D's resolution of the missing-scope-root contract.
Verified on the branch:
cargo fmt --all -- --check,cargo clippy --workspace --all-targets --locked -- -D warnings, the six delta tests by name, the retention crate's lib suite five times, the rusqlite runtime suite, and the code-index-runtime lib suite. CI is dispatched on the exact head before merge.🤖 Generated with Claude Code