fix(extraction): bind a unique trait bound as the callee - #1834
ScriptedAlchemy wants to merge 24 commits into
Conversation
A type parameter with one trait bound names Trait::method. Two bounds still abstain. Dotted calls do not regain the bare method name #1814 removed, and self still names the enclosing type. Co-authored-by: Zack Jackson <ScriptedAlchemy@users.noreply.github.com>
|
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. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 7bd29d693c
ℹ️ 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".
| if Self::annotation_is_self(state, ty) { | ||
| return Self::enclosing_receiver_type(state); | ||
| } | ||
| bounds.resolve(Self::stated_type_path(state, ty)?) |
There was a problem hiding this comment.
Resolve the method's declaring trait, not the sole bound
A sole bound does not imply that every method callable on the parameter is declared by that trait. For example, with trait Processor: Helper {} and fn f<T: Processor>(x: &T) { x.help(); }, Rust resolves help to Helper::help, but this substitution emits Processor::help; the exact owner-path resolver therefore cannot create the call edge to Helper::help. Blanket extension traits have the same issue. Preserve ambiguity or account for supertraits/method ownership instead of treating the unique written bound as the callee owner.
AGENTS.md reference: AGENTS.md:L9-L12
Useful? React with 👍 / 👎.
A history rescan reuses a coverage-only idempotency key with a new resume checkpoint. That conflict is an already-applied advance when the durable cursor is already at next, not a permanent collision that blocks ingest.
Whole-generation projection and doctor probes materialized past the exact SQL ceiling, so persist retried forever and doctor refused an oversized store before any finding. Page those reads and return the counts seen.
…n-reads fix: serve stored session reads instead of typed unavailable
#1842 merged with a rustfmt diff in observation_collision_tests.rs, so the master push run failed its formatting gate. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
A Hermes capture that the admission refuses deterministically (privacy boundary, identity collision, receipt collision) re-fails identically on every sweep. The snapshot admit loop had no durable skip for it, unlike the shared JSONL path, so the offending row pinned the source cursor and the whole `state.db` was abandoned once per sweep pass, forever: on a live daemon that was 66 WARN lines in six minutes across both Hermes profile stores with no recovery path at all. Cover past a deterministic refusal with the same typed coverage reason the JSONL admission already writes, so the source converges. Two further defects made it undiagnosable and unbearable: - host_admission_error reduced the outcome to its status family, so every refusal, cursor mismatch and contract violation surfaced as the single sentence "Hermes observation admission was degraded". Carry the reason code, retryability and storage cause the outcome already holds. - the sweep re-logged an identical WARN for a source whose state had not changed. Report a source failure when it is new or its reason changed. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> (cherry picked from commit 39a0e8538888e7c3669099da85543283ecda431d)
Live hook ingest and the scheduled catch-up sweep own the same (source, scope) observation cursor and routinely read the same transcript at once. The store's compare-and-swap keeps them honest, so one loses and gets `cursor_conflict`. The JSONL admission seam turned that into a typed block for the whole source pass, which the provider reported as a catch-up failure: on a live daemon that was 33 "Cursor transcript catch-up failed" WARN lines in six minutes for ranges the winner had already committed. The store already returns enough to decide: re-read the source cursor on a lost CAS. When the winner is on this generation and already past the frame (or, for an atomic batch, past the window's last frame), the range is durable, so adopt the winner's frontier and count the frames as skipped instead of failing. A cursor short of the frame, a different generation or an unreadable cursor all keep the existing typed block, so a frontier the winner never reached is never adopted. No store contract changes. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> (cherry picked from commit 9a7810c6dcfa7d39f70ceb009ec5d8c245b52a77)
`indexed_replay_starts_at_the_acknowledged_btree_position` resets a counter, runs one indexed replay pass and asserts it visited at most 9 B-tree entries. The counter was a process-global `AtomicU64`, so every other test replaying an index on one of the harness's other threads added to the number this test read: it measured the suite's traversal, not its own pass, and failed intermittently with no bound that held. `indexed_replay_pass` runs entirely on its caller's thread, so a thread-local `Cell` is exactly the scope the assertion means. The bound stays at 9; only whose traversal it counts changes. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> (cherry picked from commit 34d40d792041ac2d0a5311f2c033fdec3de8d1fc)
`codex_session_meta_prefix_is_decoded_once_across_consumers` asserts two profile consumers of one rollout share a single prefix decode, which holds only while the shared metadata cache still retains the entry between them. It failed intermittently for two compounding reasons. The test never installed the preparation authority its siblings install, and without one `shared_jsonl_preparation_capacity` returns the degraded fallback of a single entry, so the next publish from any parallel test evicted this path before the second consumer looked it up. Installing it was not enough. The test authority metered the whole 96-thread harness against one 32 GiB budget, and each in-flight page holds a 544 MiB reservation, so bursts drove the derived capacity down to two entries and a couple of peer publishes still evicted the entry. That ceiling is an artifact of the harness, not the product: a production process meters one ingest workload against the machine. Size the test budget past what the harness itself can reserve so capacity stays CPU-bound. No assertion or production rule changes; the eviction and capacity logic is untouched. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> (cherry picked from commit 0e1cb031afacdb27f0e144e4339e9638a01b7b37)
#1842 merged with a `drop` of a writer handle that does not implement Drop, which clippy refuses under CI's `-D warnings` lens, so the master push run failed its Clippy job. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Refresh baseline asked for 100k relations in one read. That exceeds the exact-SQL page, so materialize failed as storage busy and the running refresh never recorded progress. Page the copy count instead.
fix(session-temporal): count parent copies without loading the graph
#1842 made an idempotency conflict on a cursor already at its `next_cursor` an `ExactDuplicate` (the coverage is applied; only a conflict that left the cursor elsewhere is a collision). Two tests still expected `CursorAdvanceCollision` for the same range under a different coverage reason and failed both tries on the master push run 35428222386. Assert the new contract and keep the committed cursor. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
A refresh batch loaded 500 observation bodies in one query. Past the exact-SQL ceiling that failed as storage, and the projector reported only source_busy, so the same pass retried forever. Split the page and keep the storage error on the retry.
…h-split fix(session-temporal): split observation prefetches that exceed a page
fix: live-daemon retry storm, clone-scan stall, partial-generation zero, update identity
Refresh refused a parent generation that had rows but no applied native graph, then retried that absence as source_busy. Count canonical logical copies from the sealed rows instead.
…ation-graph fix(session-temporal): reconstruct copies without a relation graph
…4fd3' into cursor/trait-call-callee-8fe0 # Conflicts: # crates/tracedecay-code-extraction/tests/main/rust.rs # crates/tracedecay-code-index/src/chunks.rs
A receiver typed by a type parameter with one trait bound now names `Trait::method`, so the same bytes produce different reference rows than extractor.rust.v11 does. `generation_language_revisions_match` compares a sealed generation's extractor revisions against the registry, so without a bump every generation already sealed at v11 keeps the missing trait callee this change exists to add until some unrelated edit forces re-extraction. Re-pin the fixtures that carry the revision string, exactly as 1ebadc8 did for v11: - `canonical_rows_digest_matches_pinned_identity`: the revision is part of the batch identity, so the pinned rows digest moves to sha256:4e483806df. - `partitioned_codec_has_stable_bytes_and_round_trips`: the revision sits in every sealed file segment. v11 and v12 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>
Summary
T: Processor,where T: Processor,impl Processor + 'static) now namesProcessor::methodas the call-graph callee.selfstill names the enclosing type.df9f190eeaad533d00b65b19fbc0d4f47426136b). This PR does not redo that bare-receiver fix.Motivation
Bare
methodnames collide with every impl, so a trait call had no callee. #1814 stops emitting that bare name and typesselffrom the enclosing impl or trait. It does not turn a unique trait bound intoTrait::method, sovalue.process()onT: Processorstill misses the trait method. This is that binding, and nothing else. Not #1355.ci.ymlis unchanged.Changes
crates/tracedecay-code-extraction/src/rust_extractor.rs: resolve a written type parameter through its unique trait bound before emittingType::method.selfrecording from fix(extraction): stop bare receivers inventing callers #1814 is untouched.index_fileCallsedges, including thatDoubler::kickstill binds the impl method and an ambiguous bound binds nothing.Test plan
cargo test -p tracedecay-code-extraction --test main rust::— 33 passed, includingtrait_bound_calls_name_the_trait_without_a_bare_methodandbare_receiver_calls_name_self_without_the_method_simple_namecargo test -p tracedecay-code-index --lib chunks::tests::trait_bound_method_call_binds_the_trait_callee -- --exact— 1 passed (call edges toProcessor::processand the Doubler impl)cargo test -p tracedecay-code-index --lib chunks::tests::bare_receiver_method_call_does_not_invent_a_same_file_caller -- --exact— 1 passedcargo nextest run --workspace --no-fail-fastnot runcargo clippynot runChecklist
CHANGELOG.mdupdated (release automation owns this file; not edited here).envfiles included