fix(code-index): idle means the pass tail already ran - #1826
Merged
Merged
Conversation
`map_execution_error` mapped `SessionTemporalExecutionError::Empty` to
`CompleteZero` whatever freshness rode with it. After a hook ingest
commits rows, the project store publishes the temporal generation but
its relation receipt is applied later by the LCM summary convergence
page, so `root_readiness` answers `Partial { generation_lag: 1 }` and
the candidate cohort is empty. The first `tracedecay_message_search`
then reported `outcome: complete_zero` — "nothing exists, and that is
final" — for a transcript the store had already committed.
`map_report` already refuses that for an empty ranked page: a `Partial`
generation returns the typed partial outcome instead. The
execution-error path owed the same refusal. It now answers
`Partial { items: [], omitted: generation_lag }`, which the retained
message-search surface already renders as `outcome: "partial"`, so a
caller re-reads rather than believing the zero.
`production_codex_hook_ingest_survives_message_search_reopen` loops on
that typed partial the way the refresh assertions loop on `running`,
and still fails on an empty `complete_zero` or any other outcome.
Measured on `taskset -c 0,1`, 6 concurrent copies, 42 runs:
25 failures before, 0 after.
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
`tracedecay update` installed v0.1.0-beta.47, restarted the daemon, then refused the binary it had just installed: protocol identity mismatch (name=tracedecay, version=0.1.0-beta.47+84598a0b..., expected version=0.1.0-beta.47) and the daemon logged `daemon_version_skew` on every readiness poll, with the same two strings. Both sides were one binary. One value drove both symptoms. The GitHub-release upgrade path reports the release it installed as the bare tag (`upgrade.rs` `run_versioned_upgrade`), while the package-manager path reports the installed binary's own `--version`. That value becomes the maintenance window's `expected_version` (`service.rs` `adopt_maintenance_outcome`), which readiness compared to the daemon's advertised `build_version()` with a raw string `==`, and which `query_daemon_identity_stream` also sends as the probe's own `client_version`, where `client_version_skew` compared it with a second raw `==`. Build metadata never changes SemVer precedence, so a side reporting only the release is less specific, not different. `versions_name_same_build` is now the one comparison every identity check runs: equal release precedence, and the same commit whenever both sides name one. That keeps the skew 367a44a added this comparison to catch, two checkout builds of one release differing only by commit, while the release tag and the binary it ships resolve to one identity. Tests cover both halves and fail without the fix: readiness classifies 0.1.0-beta.47+<sha> as Ready against a bare 0.1.0-beta.47, a stale release and a second commit of the same release stay mismatches, and unparseable versions still compare literally. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
A daemon spent eleven hours emitting 70,985 identical warnings:
session temporal refresh pass will retry class=Storage
retry_attempt=25107 error="Storage { operation: \"persist session;
refresh progress\", source: Sqlite { code: Some;
( 19 );
, extended_code:;
Some;
( 1811 );
, message: \"invalid session refresh progress\" } }"
Three workers each resubmitted the same progress row about once a
second for the whole process lifetime, pinning two tokio workers.
Two independent defects combined:
1. The worker used `SessionStoreError::is_storage` as its retryability
predicate. That only says the failure came from the storage adapter.
A `SQLITE_CONSTRAINT` abort is a schema-contract trigger refusing
this exact row, and an exact-SQL materialization ceiling (6,627 more
warnings on the same loop, reported as an untyped `Runtime` message)
refuses this exact statement. Replaying either unchanged can only
spin at the backoff cap, which for `Storage` is 800ms forever.
2. Even classified terminal, the projection arm only counted the error
and left the operation `running`, so the next pass rediscovered it
and rebuilt the same row. Only the projector's terminal errors
durably failed a refresh.
`Error::is_deterministic_refusal` now names the two engine failures
that cannot be replayed, `QueryLimitExceeded` maps to the typed
`InvalidOperation` its sibling limit refusal already used, and the
worker retires a refused refresh through the existing terminal-attempt
and discovery-suppression machinery.
`validate_successor` also admitted an equal committed frontier where
the durable guard requires a strict advance, so a stalled successor
reached the trigger as a constraint abort instead of typed state. It
now matches the guard.
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
A daemon mounted /fast/projects/tracedecay at 15:25:35, decided the
sealed generation predated the build and had to be rebuilt, and then
recorded no further scheduler progress for eleven hours. A full thread
dump taken while it was stuck shows two blocking-pool threads in
registry::mount_worktree_inner
-> LatestCodeTextGenerationV1::advance_text_serving_inner
-> CodeLexicalCloneSuccessorV1::verify_resumed_page
-> verify_clone_fingerprint_page_rows
-> sqlite3_step / sqlite3BtreeNext / readDbPage
so the mount was not blocked on a seat, a permit or a store lock: it was
running SQL, which is why the process also sat at 100% on two workers.
`clone_exact_postings` and `clone_fingerprint_postings` are WITHOUT
ROWID tables whose primary keys start at `class` and `language`, so
`WHERE symbol_occurrence_id = ?` has no index and scans the whole table.
Resume verification issued two of those per clone body, making the pass
quadratic in a repository's postings.
Both tables are now read once per page and bucketed by occurrence. Each
primary key is unique within one occurrence, so a sorted bucket
reproduces the `ORDER BY` the per-body queries used and the comparisons
stay byte-identical; `clone_body_payloads` and `clone_occurrences` are
already keyed by their lookup column and are untouched. Resume
verification is covered by
`v16_clone_payloads_are_content_addressed_and_postings_page`, which
drops and reopens a successor mid-corpus and then compares finished
section digests.
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
cc10394 made every non-retryable failure of the projection batch persist retire the running refresh. A cancelled worker control also fails that persist, and it is not a refusal: the row was never submitted, and the next pass may hold a live control. Retiring it through `claim_terminal_attempt` under a cancelled state claimed nothing and counted nothing, so `cancelled_worker_control_prevents_projection_batch_persistence` observed zero terminal errors on CI. Only a deterministic engine refusal (constraint abort, exact-SQL ceiling) retires the refresh; every other terminal error counts as before and leaves the operation running. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
8b75523 cut resume verification from one postings scan per body to one per page, but a restart still replays every committed page, so a daemon resumed against a large repository sat inside `verify_clone_page_rows` for over thirty minutes with no scheduler progress: N pages times a full scan of both postings tables. The clone successor now installs `symbol_occurrence_id` indexes on both postings tables when it opens (idempotent, so a copied prior built without them gains them), and page verification reads postings per occurrence through them. Nothing digests or enumerates the index schema; the receipt records the file size after the indexes exist. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
`single_analytics_append_commits_in_one_writer_dispatch` asserted the append future was still `Poll::Pending` after its first poll. That poll enqueues the INSERT on the writer OS thread via a synchronous `try_send` and then awaits a `bounded(1)` reply channel, so the writer can execute, autocommit and answer before the caller first polls the receiver. The assertion was racing that thread, not checking a contract: 9/60 failures under `taskset -c 0,1` with 6 concurrent copies, matching the CI flake in run 35422336760 (TRY 1 fail, TRY 2 pass). The contract commit 4fc1439 added is that a *detached* append still commits: the writer runs the request and only then replies, ignoring a dropped receiver. Make the test enforce that with a gate it owns -- scope the future so it is dropped right after the single poll, before anyone reads the reply -- and keep the existing commit and no-retained-transaction checks. Restoring the transaction-framed append fails the restructured test 30/30 with "single append did not autocommit", so the regression guard is intact and now deterministic. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> (cherry picked from commit ea8113159372c2f4ab1f7c1e8138ce4c5d846494) (cherry picked from commit 80341240b4e94c5fbe075d024337c87b256cc3f3)
`mounted_code_generation_retention_continues_capped_segment_reclamation`
failed both nextest tries at ~3.9 s on master run 35422072661 with `code
generation retention plan: Storage("No such file or directory (os error
2)")`; the identical tree passed the same partition minutes earlier on run
35420910183, so it is a race. Under `taskset -c 0,1` with 6 concurrent
copies it reproduced 4/78.
Instrumenting every enumerate-then-open loop in the planner with the path
and a re-stat named the vanishing file on both ENOENT reproductions: the
text-artifact inventory's `read_dir` + `symlink_metadata` loop, on
`.text-artifact-<digest>.staging` and on a `.staging-journal` sidecar,
both `present_now=false` while their siblings were still listed.
`prepare_next_code_generation_retention_cancellable` scans without the
generation-store lock on purpose - the full-digest read routinely covers
several GiB and must not pin the daemon writer gate - and the text-artifact
builder retires a whole `.staging` family (`discard_incompatible_staging`
-> `retire_text_artifact_staging_family`) under that lock in the background
pass tail, the same tail fenced in 656b532 and 760e221dda. So an entry
the listing just named can be gone before the scan stats it, and the whole
plan failed. Production reads that as `retention_plan_failed` and fails the
pass with a loud degraded log, so every publish that raced a maintenance
tick lost the tick.
An entry that vanished mid-scan is already reclaimed, which is what the
inventory would have planned for it anyway: skip it, the way this loop
already skips an absent active-staging path fifteen lines above. Completed
artifacts the durable index references are verified before this scan and
stay fail-closed. The unit test retires the staging family from inside the
scan's own cancellation probe and asserts the plan survives without naming
the vanished files; it fails with the ENOENT before this change.
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
(cherry picked from commit 012965c52514790e56d5531e51b48acb808cd1c5)
The same 6-copy `taskset -c 0,1` reproduction of `mounted_code_generation_retention_continues_capped_segment_reclamation` also failed with `code generation retention plan: GenerationStoreBusy` (1 of 4 failures in 78 runs). That is not a failure. The planner's recovery probes the generation-store lock with `try_acquire_code_generation_store_lock` and answers `GenerationStoreBusy` when a writer owns the store; production maintenance consumes it with `defer_generation_store_busy` and comes back on the next tick. The route under test stays mounted, and `publish_code_edit` returns as soon as the serving generation id changes, so the pass tail that sealed that generation can still own the store when the test plans - the same pass-tail exposure as 760e221dda. Consume the typed busy answer under a bounded wait instead of reading it as fatal. Every assertion on the resulting plan is unchanged, and any other error still panics. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> (cherry picked from commit 833a3f30a2c77c426864123c0065bc4d8b664586)
`tracedecay_unsafe_patterns` picked `enclosing` as the smallest-line-span
symbol covering the match's line. When several declarations share one line
every candidate has span 1, so the winner was whichever symbol the page
happened to yield first, and pages arrive in occurrence order over
per-project digests. `#[test] fn a() { x.unwrap(); } pub fn b() { panic!(); }`
therefore reported `@test` (the annotation-usage node the Rust extractor
names `::@name`), `a`, or `b` at random: 7 of 12 identical local runs
disagreed, which is what broke the CI job deterministically on its runner.
Resolve the enclosing declaration by the byte range the graph already
publishes in `CodeGraphSymbolBindingV1::source_span`, containing the offset
of the matched construct. That is stable, names the innermost declaration
that really contains the site, and drops attributes for free: `#[test]`
spans only its own bytes, so it can never contain a call.
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
(cherry picked from commit 178afe414b370165fd8910620085e922d7e9fe27)
`adapter_leaves_repository_byte_identical` snapshots `.git` before and after the read-only adapter calls. The fixture's `git commit` spawns a detached `git maintenance run --auto`, whose `.git/objects/maintenance.lock` was still present for the first walk and gone for the second, so the byte-identical assertion failed on both nextest tries with the lock as the only difference. Disable auto maintenance and gc for every fixture git call, as the scheduler and daemon fixtures already do. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
An explicitly published branch worktree is never a project-open route, so nothing mounted a core query authority on it. An exact branch read could only borrow one already mounted on a peer checkout of the same repository (`mount_query_authority_from_project_peer`), and that peer's own mount is deferred until it seats a text generation. A read taken right after the publication sealed its provenance therefore failed closed with a non-retryable `authority_unavailable`, even though the branch generation was committed and servable. That is the CI flake in the tracedecay-cli `core_cli_suite` linked-worktree branch-add journey, where a freshly added peer worktree searches the just-published branch before it has seated anything of its own. Mount the published branch's own authority from the project's durable cursor-key authority, reusing the existing project-open mount rather than adding a second mount path. It runs inside the daemon-owned publication task after the generation is committed, so the admitting `branch add` caller — which returns at admission, before activation even starts — never waits for it and never pays for the profile session-registry lock. An earlier attempt (51402cd) mounted on the admitting caller's path and correlated with mount and publish failures elsewhere; this one stays off that critical section. Best effort by design: the generation is already committed, so a missing session mount or cursor key must not retract it, and the exact read still falls back to borrowing a peer authority when the mount could not run. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> (cherry picked from commit 6d887a0c3ce25c2838f2943200e133a610cdf2e8)
The worker dropped its admission permit and reconcile_in_progress, then still renamed the active pointer and stamped BusyFollowUp. Those two signals are now idle only after that tail: continuations are stamped while the pass is visible, and published text projection re-takes the permit before the pointer rename. The pass is dropped only across that permit wait, so a holder waiting on the flag cannot deadlock. Co-authored-by: Zack Jackson <ScriptedAlchemy@users.noreply.github.com>
|
ScriptedAlchemy
marked this pull request as ready for review
September 19, 2026 06:37
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: 2579de9da1
ℹ️ 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
+2123
to
+2124
| let _visible = super::ReconcilePassGuard::enter(passes); | ||
| Self::note_worker_continuation(pending_wake, wake); |
There was a problem hiding this comment.
When the worker has dropped its main pass guard before optional graph work (for example at mount.rs:1233-1235), a caller can acquire the admission permit, observe both reconcile_in_progress == 0 and an empty pending slot, and conclude that the owner is settled before this helper is invoked later by graph preparation or serving-swap logic. Entering a guard only for the duration of note_worker_continuation cannot repair that earlier quiet window: the caller may already have returned before the temporary guard is created, after which this code stamps a new BusyFollowUp. The pass must remain visible, or the continuation must be reserved, from before the unguarded tail begins rather than only while the note is written.
Useful? React with 👍 / 👎.
`tracedecay_field_sites` picked each site's `enclosing` declaration by filtering the file's symbols to those whose line range covers the site's line and taking the smallest `line_span`. When several declarations share one line every candidate has span 1, so `min_by_key` broke the tie on the order the graph page yielded symbols, and pages arrive in occurrence order over per-project digests: a coin flip per run, the same defect 63c0784 fixed in `tracedecay_unsafe_patterns`. Resolve `enclosing` by the byte range the graph already publishes in `CodeGraphSymbolBindingV1::source_span` containing the site's own byte offset, innermost containing span winning. `FieldSite` already carried that offset (`byte`, the end of the field name, keyed the same way as the receiver-type map), and masking preserves byte layout, so no matcher change was needed. `enclosing_declaration` moves from `unsafe_patterns` up to the shared `analysis` module and now returns the symbol rather than its name, since the field-site qualifier check needs the occurrence id. Both handlers then apply one attribution rule instead of two copies that can drift. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> (cherry picked from commit 71522df546dae3b7cff729d60988b2f7bb13b750)
`elapsed_freshness_window_alone_does_not_make_dashboard_state_stale` read the dashboard right after `wait_for_dashboard_ready`, which joins only the running pass. The mount leaves clone backfill behind, and the wakes that drain it leave a banked permit whose no-op pass projects `Verifying` instead of `Fresh` at the sample (CI run 35425541839, both tries). Use the single-permit registry, settle the mount-era chain, hold the admission so no pass can start, and prove the pending-wake slot stays empty, as the text-progress test already does. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Under `--all-features` the binary binds the hotpath metrics port on start. When a sibling test's daemon already holds it, the bind error lands on stderr and `shipped_binary_stops_quietly_when_a_pipeline_reader_exits` fails on its empty-stderr assertion. Turn the metrics server off for this command, as `hotpath_command` already does. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
…or/scheduler-admission-races-2703
The published text projection re-acquired the background admission permit
before renaming the active pointer, while `_build_publication` was still
held for the rest of the worker iteration.
`run_ignored_dependency_admission` takes the same admission *before* that
per-worktree gate, so the two orders invert: an ignored-dependency owner
holds the permit and waits for the gate, the worker holds the gate and
waits for the permit. That stalls the publication pass until the
dependency request hits its own deadline and refuses, and with every
permit consumed it is a cycle. The repository already states the
invariant the re-acquire broke:
`background_worker_waits_for_global_admission_before_publication_gate`
("global admission wait must not hold the per-worktree publication gate").
The re-acquire also bought nothing for this PR's claim. `reconcile_pass`
is already held across the whole published text projection, so the
pointer rename was inside the pass an idle reader samples; dropping the
pass across the new permit wait instead opened a fresh window where
`reconcile_in_progress` reads zero before the rename has run. Drop the
re-acquire and keep the pass guard continuous, which is what makes
"idle means the pass tail already ran" true here.
The continuation-ordering half of the change is untouched: stamps still
land before the pass goes idle.
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
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
reconcile_in_progress == 0and a free admission permit) while it still stampsBusyFollowUpor renames the active pointer.pull_requesttriggers in.github/workflows/ci.ymlare untouched.Motivation
On tip
3f71bccb2dbathe background worker released its admission permit before HeadOpening and droppedreconcile_in_progressbeforenote_worker_continuation. Every later fence (including #1794) assumed those two signals meant the pass was finished. They do not. The worker is the actor that publishes the false quiet, then writes the pointer and stamps the wake.Dispatch SHA:
2579de9da1e39d66f28ba4b2ad6415c62c1ecd94Changes
crates/tracedecay-code-index-runtime/src/code_index_scheduler/registry/mount.rs: stamp known continuations before the pass goes idle; graph-discovered stamps happen under a pass guard before that step is observed; published text projection re-acquires admission after head-opening's scheduler-mutex wait and beforedrive_text_projection.registry.rs:note_visible_worker_continuationenters the pass for the stamp itself.tests/mod.rsno longer describes the dropped-guard tail as the reason the slot has to be polled.Test plan
cargo test -p tracedecay-code-index-runtime --libon the ten scheduler tests that observe pass tails and admission (all passed).concurrent_query_admissions_claim_one_pending_wake_before_worker_coalescing10/10.cargo clippynot re-run for the workspace.Checklist
.envfiles includedCHANGELOG.mdupdated (scheduler internal ordering; no user-facing contract change)