fix(extraction): stop bare receivers inventing callers - #1814
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>
Dotted Rust calls emitted the method's simple name, so a unique same-file callable of that name became a caller. Bind self through the enclosing impl or trait, and keep Type::method for stated receivers. Co-authored-by: Zack Jackson <ScriptedAlchemy@users.noreply.github.com>
|
ScriptedAlchemy
marked this pull request as ready for review
September 19, 2026 06:30
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. |
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)
![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: df9f190eea
ℹ️ 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".
| { | ||
| None | ||
| } else { | ||
| Some(type_name.to_owned()) |
There was a problem hiding this comment.
When an impl or trait is declared inside an inline module, this returns only the owner name, so mod inner { impl Rows { fn measure(&self) { self.len(); } } } emits Rows::len while resolve_file_references indexes the definition as inner::Rows::len. Qualified names are exact-matched there, and sealing discards same-file targets; because this commit also removes the fallback bare len reference, the call now produces no Calls edge. Preserve the enclosing module segments when constructing the receiver type.
Useful? React with 👍 / 👎.
Comment on lines
+1678
to
+1680
| match name | ||
| .strip_prefix('<') | ||
| .and_then(|inner| inner.split_once(" as ")) |
There was a problem hiding this comment.
as in projected impl types
For a valid projected self type such as impl LocalTrait for <Foo as Assoc>::Item, the stored owner is <<Foo as Assoc>::Item as LocalTrait>, so this unconditionally uses the first as and returns <Foo. Calls through self consequently emit malformed names such as <Foo::method and cannot bind to the extracted implementation method. Find the outer depth-one as delimiter, as the indexer's existing UFCS parser does, rather than splitting at the first occurrence.
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/false-edge-bare-receiver-4fd3
8 tasks
Owner
Author
|
Follow-up trait-bound callee binding: #1834. |
`enclosing_receiver_type` returned the bare impl or trait owner, so a
`self.len()` inside `mod inner { impl Rows { .. } }` emitted `Rows::len`
while the definition is indexed under its whole file-relative name,
`inner::Rows::len`. `resolve_file_references` exact-matches that key, so
with the bare method-name duplicate gone the call bound nothing at all
and was retained as a phantom cross-file reference instead.
The receiver type now carries the frames between the file root and the
impl, which is exactly how the method's own qualified name is built.
The stored owner of a trait impl is `<Type as Trait>`, and `Type` can
itself be a projection (`<Foo as Assoc>::Item`), so splitting at the
first ` as ` yielded `<Foo`. Split at the ` as ` at depth zero, the
same delimiter the indexer's UFCS parser already uses.
Both defects were reported by the Codex review on PR #1814.
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
`rust_type_path_alias_for_trait_impl_method` required the file-relative name to start with `<`, so a trait impl inside an inline module (`inner::<Rows as Wide>::wide`) got no `inner::Rows::wide` alias. That was invisible while every dotted call also emitted its bare method name; now that a call names its receiver type, `self.wide()` in that module bound nothing. The parser now peels an enclosing module path before the UFCS head and restores it on the alias, so the guard that lets an inherent method keep the path and `rust_qualified_name_is_trait_impl_method` see the same shape both keep working for module-scoped impls. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
The extractor no longer emits the bare method name of a dotted call and now types `self` from the enclosing impl or trait, so the same bytes produce different reference rows. `generation_language_revisions_match` compares a sealed generation's extractor revisions against the registry, so without a bump every already-sealed generation keeps the false same-file callers this change exists to remove until some unrelated edit forces re-extraction. Re-pin the three fixtures that carry the revision string: - `canonical_rows_digest_matches_pinned_identity`: the revision is part of the batch identity, so the pinned rows digest moves to sha256:e92b7ad8f9. - `partitioned_codec_has_stable_bytes_and_round_trips`: as in acc2773 ("re-pin partitioned codec onto extractor.rust.v10") the revision sits in every sealed file segment. v10 and v11 are the same length, so all four segment sizes are unchanged (11_071, 5_171, 6_279, 6_837) and only the digests move: an identity change, not container drift. - The two worker tests and the reconcile test that assert the current revision after a forced re-extraction. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
ScriptedAlchemy
added a commit
that referenced
this pull request
Sep 19, 2026
Batch run 35431957131 failed report_tests::baseline_report_retains_ raw_fallback_current_and_exact_ten_x_samples and the CLI compare journey: the query-fallback train/validation receipts drifted from the pins #1828 recorded on 3f71bcc. The drift is a ranking change, not a generation reseal: it reproduces on this branch and disappears with PR #1814's three extraction commits reverted (1ebadc8, 3daba38, df9f190: bare receivers no longer invent same-file callers, module scope kept in self receiver types, extractor.rust.v11), so the lexical-graph workload's ordered rows legitimately moved with the extracted call graph. The fixed conceptual-miss set asserted by the same test is unchanged (train-015..033, validation-015..025). 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
extract_call_sitesfrom emitting the method simple name of a dotted Rust call. That name was resolved as a same-fileCallsedge whenever one local callable shared it (items.push()→fn push).self(andSelfin an annotation) through the enclosing impl or trait type, soself.len()still namesRows::len. Stated receivers keepType::method.8c36e0ad0aand0f0769d331are not reachable on this repository;71be4d0ca5is excluded.ci.ymlis unchanged (nopull_requesttrigger).Motivation
Same-file resolution treats an unambiguous callable name as a real call. The extractor was inventing that name for every
receiver.method()so a unique localfn methodorimplmethod became a caller, including untyped receivers the typed-path work already refuses to guess. The root is the extra unresolved ref, not a later guard.Dispatch SHA:
df9f190eeaad533d00b65b19fbc0d4f47426136bChanges
crates/tracedecay-code-extraction/src/rust_extractor.rs: drop the bare method-name duplicate; typeself/self: &Selffrom the enclosing impl or trait (<Type as Trait>still yieldsType).index_filecall edges.Test plan
cargo test -p tracedecay-code-extraction --test main rust::— 32 passed, includingbare_receiver_calls_name_self_without_the_method_simple_namecargo test -p tracedecay-code-index --lib chunks::tests::— 38 passed, includingbare_receiver_method_call_does_not_invent_a_same_file_caller(exact call-edge list)df9f190eeaad533d00b65b19fbc0d4f47426136b(CI has nopull_requesttrigger)Checklist
CHANGELOG.mdupdated (release automation owns this file; not edited here).envfiles included