chore: integration batch D (#1834, #1836, deslop, two #1848 follow-ups) - #1860
Merged
Merged
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>
A seat waiter that sees reconcile_in_progress at zero treats the owner as finished. Noting BusyFollowUp after that drop sampled an empty slot and then raced the failure ceiling. The stamp now holds the pass guard for the note only. A store directory that does not exist yet is an unpublished plan, not Storage(NotFound). Planning against latest_generation_id before cold open created the scoped store was that error. The retention journey now re-reads the store on the serving-seat signal. Co-authored-by: Zack Jackson <ScriptedAlchemy@users.noreply.github.com>
…4fd3' into cursor/trait-call-callee-8fe0 # Conflicts: # crates/tracedecay-code-extraction/tests/main/rust.rs # crates/tracedecay-code-index/src/chunks.rs
…-failure-ceiling-3ef6 # Conflicts: # crates/tracedecay/src/daemon/production_harness/generation_retention_test.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>
Two comments added by this branch's own integration commits restate their commit bodies in the source. codex.rs: the revert of #1812 (7d678d8) pasted eleven lines of the rollback/StalePreview post-mortem above the deferral it restores. The revert's message already holds it, and the branch's own unit guard `prepare_defers_when_no_plugin_cli_resolves` is what keeps the deferral from regressing. The function is now byte-identical to pre-#1812 master. core_client.rs: the doc on `is_project_open_retryable_error` (e6f94cd) ended by naming the reset-recovery journey and the error string it emitted. That couples the comment to a test name nothing enforces. The sentence before it already states why the open subset alone is wrong, and the unit test in the same file pins the revoked retry. No behavior change. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Binds a receiver typed by a type parameter with one trait bound to Trait::method during Rust extraction, and moves the Rust extractor revision from v11 to v12 with the pinned rows digest that follows it. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Stamps a worker continuation while `reconcile_in_progress` still reports the pass, so a seat waiter that sees the counter reach zero cannot sample an empty slot and then race the failure ceiling. Treats a scoped store directory that cold open has not created yet as an unpublished plan rather than `Storage(NotFound)`. Conflict resolutions against the scheduler work batch C already carries from #1826, #1821 and #1811: - `code_index_scheduler/registry.rs`: identical `note_visible_worker_continuation` body on both sides, only the doc prose differed. Kept batch C's landed wording; rewording a landed comment is churn with no behavior change. - `code_index_scheduler/registry/mount.rs`: six hunks. Four were comment wording for identical code. The other two are the same stamp at different positions: batch C stamps the retained-recovery successor before the recovery await, and the roster-refusal rebuild right after `take_ignored_roster_refusal_rebuild` through the `prepare_*` handles, while this PR stamps both later and re-enters a pass guard to do it. Kept batch C's earlier placement, which holds the guard across a strictly wider window and so satisfies the invariant this PR is after. No production line of this PR is lost by that choice. - `code_index_generations/tests.rs`: both sides added an independent test at the same offset. Kept both, batch C's `vanished_listed_generation_open_defers_instead_of_storage_loss` and this PR's `missing_store_is_an_unpublished_plan_not_a_storage_failure`. - `production_harness/generation_retention_test.rs`: took the union. This PR's canonical project root, serving-seat wake and convergence predicate all stay, because the predicate waits for the superseded source to actually be collectable instead of accepting the first plan the planner returns. Batch C's `GraphReplayPoolBusy` arm is folded into the deferral match, so that typed answer is still consumed rather than panicking the test. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Drops the commit-message prose the batch-c fixes inlined as comments. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Picks up batch C's final tip: the global-db page rebuild overlap reconciliation from fix/batch-c-codex-triage. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
`Path::is_dir` follows symlinks, so a pointer slot replaced by a symlink to any directory would satisfy it and hide exactly the defect this test guards: publication overwriting a slot it never observed. `read_optional_active_pointer` classifies the slot with `std::fs::symlink_metadata`, so the test now reads it the same way the production store does, and fails outright when the node is gone. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
`runtime_mounting_problem` and `project_open_refusal_response` each spelled out the same `RUNTIME_MOUNTING_REASON_CODE` diagnostic and message. That is the one code the one-shot client re-sends on, so two hand-written copies could drift apart and silently split the mounting window from the answer a client is allowed to retry. Both now call `ApplicationProblem::runtime_mounting()`. The tracedecay-mcp test that constructs the same wire value by hand stays as it is: it is the input fixture its own assertion is checked against. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
`runtime_mounting`'s second sentence argued for the refactor that created it rather than stating a contract, and the retention journey's comment spent nine lines narrating a race the loop below it already shows. Both keep only the part a reader cannot recover from the code: what the constructor answers, and why the first plan the planner returns is not the one the test wants. The comment audit also proposed a `CanonicalProjectRoot` newtype, a retyped `prepare_next_code_generation_retention_cancellable` store root, and relocating the extractor revision onto `RustExtractor`. All three are design changes to PRs that were reviewed as they stand, so they are left for their own change rather than folded in here. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Master e130045 sync and the describe-preview test expectation. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Clone-lane discard fix and the narrowed test wait. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.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. |
![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: a724f58e74
ℹ️ 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".
| /// path, including the enclosing type recorded for `self`, is unchanged. | ||
| fn resolve(&self, path: String) -> Option<String> { | ||
| match self.parameters.get(path.as_str()) { | ||
| Some(ParamBound::Unique(bound)) => Some(bound.clone()), |
There was a problem hiding this comment.
A unique bound does not imply that the bound itself owns every callable method. For valid Rust such as trait Derived: Base {} with fn f<T: Derived>(value: T) { value.process() }, where process is declared by Base, this rewrites the receiver to Derived and emits Derived::process; resolve_file_references only looks for that qualified name, so it cannot bind the actual Base::process symbol. Account for inherited trait methods, or abstain when ownership cannot be established from the bound alone.
Useful? React with 👍 / 👎.
Comment on lines
+825
to
+826
| Err(error) if error.kind() == std::io::ErrorKind::NotFound => { | ||
| return Ok(unpublished_store_plan(vector_readable_sources)); |
There was a problem hiding this comment.
When vector_readable_sources is nonempty, returning this empty Full plan bypasses the planner's existing missing-source integrity check and claims that generations absent with the entire store were verified. In the production caller, a store removed after the initial is_dir() check but before the blocking planner therefore produces has_collectable_work() == false and a Complete maintenance outcome rather than reporting lost pinned data; a cold unpublished store should be represented as a retryable/typed state instead of an empty success whenever protected sources are already named.
AGENTS.md reference: AGENTS.md:L192-L193
Useful? React with 👍 / 👎.
Two landed changes disagreed about a store root that does not exist yet. #1811 deferred with `GenerationStoreBusy`, #1836 returned the unpublished plan. Both tests survived the batch-D merge, so `preparation_defers_when_the_scope_root_does_not_exist_yet` failed deterministically against #1836's early return. Keep the unpublished plan. The only production caller is `tracedecay-maintenance::store_maintenance::run_code_generation_retention`. It routes `GenerationStoreBusy` through `defer_generation_store_busy`, which logs `retention_degraded failure=generation_store_busy` and returns `Failed`. `generation.rs` turns `Failed` into `MaintenanceTickOutcome` `Retry`, which `tick.rs` schedules on the short `retry_delay` and counts in `daemon.maintenance.generation.retry_total`. That deferral is a counted, logged failure, not a quiet wait. An empty plan returns `Complete`, which `tick.rs` schedules on the ordinary `interval`. Neither answer is terminal, so no caller plans once and stops. The maintenance loop re-plans every tick, and the mounted retention journey loops on `Ok(_)` and `GenerationStoreBusy` alike. The difference is duration. An absent scope root has no publisher to wait for, because cold open creates it inside the worker, so a mounted project that is never indexed keeps an absent root for the life of the daemon and the deferral would report degraded on every tick. The unpublished plan still converges the moment the store appears. The failure ceiling #1836 names is `SERVING_SEAT_FAILURE_CEILING`, a test-only wall bound on a positive serving-seat wait in `code_index_scheduler/tests/mod.rs`. It consumes no retention error, so it constrains neither contract. #1811's deeper `NotFound` mapping stays. `deferred_if_absent` still covers the two enumerate-then-open sites, where `read_dir` already proved the name existed and a peer can unlink it before the open. That race is unreachable from the root check, and d6d8665 already reverted the two durable-state sites on the same reasoning. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
A missing scope root is an unpublished plan (#1836), not GenerationStoreBusy (690e843): the only production caller re-plans on its interval either way, and an absent root has no publisher to wait for, so Busy would log degraded and retry fast forever for a never-indexed project. The enumerate-then-open deferrals stay. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
8 tasks
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 for the next verified batch, merged with
--no-ffso each folded PR keeps its history and GitHub marks it merged.Folded:
Follow-ups to #1848:
symlink_metadata, matching how the publication store checks slots, so a symlink to a directory cannot satisfy it.ApplicationProblem::runtime_mounting()builds the mounting refusal in one place for both daemon producers.Verified on the branch before the master sync:
cargo fmt --all -- --check,cargo clippy --workspace --all-targets --locked -- -D warnings, the touched unit and daemon-suite tests, the #1834 and #1836 tests, and the retention journey eight times. CI is dispatched on the exact head before merge.🤖 Generated with Claude Code