fix(cargo-gamma): stabilize builds and output handling - #163
Conversation
There was a problem hiding this comment.
🔵 Needs a closer look
It changes core staged-build convergence/scoping behavior and build invocation strategy in ways that are difficult to fully validate without human end-to-end review.
Pull request overview
This PR improves cargo-gamma’s staged build execution and user-facing build output handling to reduce redundant compilation work, make output capture more resilient for large builds, and ensure progress rendering is clean on non-ANSI/redirected displays.
Changes:
- Stabilizes staged build feature unification by keeping a whole-workspace Cargo root set when the resolved selection covers all workspace members, while constraining mutation blame/rollback to the current stage.
- Makes build output capture more robust by raising per-stream and per-line limits and switching to demand-driven buffer growth.
- Normalizes Cargo progress redraw rendering by consuming Cargo’s leading erase-to-end-of-line control sequence rather than emitting it as visible text.
File summaries
| File | Description |
|---|---|
| crates/cargo-gamma-lib/src/report/progress.rs | Strips Cargo’s leading ESC[K redraw prefix before encoding progress output; adds a regression test. |
| crates/cargo-gamma-lib/src/exec/measure.rs | Adds logic to detect when staged checks should retain whole-workspace roots based on Cargo’s resolved selection; removes containment startup warning emission. |
| crates/cargo-gamma-lib/src/exec/build/tests.rs | Updates probe behavior expectations and adds tests ensuring blame/deferral are scoped to the stage being judged. |
| crates/cargo-gamma-lib/src/exec/build/invoke.rs | Raises output limits and avoids reserving maximum buffer sizes up front by using dynamic growth. |
| crates/cargo-gamma-lib/src/exec/build.rs | Introduces scoped convergence (separating Cargo roots vs. mutation attribution scope), stage-scoped blame filtering, and workspace-staged checking behavior. |
| crates/cargo-gamma-lib/docs/DESIGN.md | Updates the design contract to reflect progress handling, output limits, and staged whole-workspace root behavior. |
Review details
- Files reviewed: 6/6 changed files
- Comments generated: 0
- Review effort level: Lite
💡 Configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Codecov Report❌ Patch coverage is ❌ Your project status has failed because the head coverage (97.5%) is below the target coverage (100.0%). You can increase the head coverage or adjust the target coverage. Additional details and impacted files@@ Coverage Diff @@
## main #163 +/- ##
=======================================
- Coverage 97.5% 97.5% -0.1%
=======================================
Files 300 300
Lines 67799 68538 +739
=======================================
+ Hits 66160 66872 +712
- Misses 1639 1666 +27
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
e885edc to
ff32310
Compare
There was a problem hiding this comment.
🟡 Changes recommended
It mixes crate/version bumps into a fix PR and also introduces an output-buffer growth approach whose allocation behavior may exceed the stated limits unless clarified or further constrained.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Review details
- Files reviewed: 15/16 changed files
- Comments generated: 3
- Review effort level: Lite
ff32310 to
7ae5921
Compare
There was a problem hiding this comment.
🟡 Changes recommended
It includes a staged-build scoping issue that can let out-of-scope mutants affect whole-workspace stage builds, and it bundles crate version bumps that the repo’s release process expects to land in dedicated release PRs.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Review details
Suppressed comments (2)
Cargo.toml:41
- These workspace dependency entries were bumped to 0.2.1. The repo’s release docs describe version bumps as part of a release PR driven by scripts/release-crate.ps1 (README.md:68-86); consider keeping these changes out of this fix PR and doing them in the release PR that publishes cargo-gamma.
cargo-gamma-attrs-impl = { path = "crates/cargo-gamma-attrs-impl", default-features = false, version = "0.2.1" }
cargo-gamma-engine = { path = "crates/cargo-gamma-engine", default-features = false, version = "0.2.1" }
cargo-gamma-lib = { path = "crates/cargo-gamma-lib", default-features = false, version = "0.2.1" }
cargo-gamma-process = { path = "crates/cargo-gamma-process", default-features = false, version = "0.2.1" }
cargo-gamma-rt = { path = "crates/cargo-gamma-rt", default-features = false, version = "0.2.1" }
Cargo.toml:42
- This workspace dependency entry was bumped to 0.2.1. The repo’s release docs describe version bumps as part of a release PR driven by scripts/release-crate.ps1 (README.md:68-86); consider keeping these changes out of this fix PR and doing them in the release PR that publishes cargo-gamma.
cargo-gamma-unsafe = { path = "crates/cargo-gamma-unsafe", default-features = false, version = "0.2.1" }
- Files reviewed: 15/16 changed files
- Comments generated: 9
- Review effort level: Lite
7ae5921 to
a363479
Compare
There was a problem hiding this comment.
🔵 Needs a closer look
It includes crate/workspace version bumps that (per the repo’s documented publishing flow) should be done in a dedicated release PR rather than bundled into a fix PR.
Review details
Suppressed comments (10)
crates/cargo-gamma/Cargo.toml:7
- This PR bumps the crate version, but the repo’s documented publishing flow expects version bumps to be done in a dedicated release PR after the desired changes are already on main (see README.md “Publishing Crates”, which drives version changes via scripts/release-crate.ps1). Please revert the version bump here and do the bump as part of the release PR instead (which will also update Cargo.lock).
version = "0.2.1"
crates/cargo-gamma-unsafe/Cargo.toml:7
- This PR bumps the crate version, but the repo’s documented publishing flow expects version bumps to be done in a dedicated release PR after the desired changes are already on main (see README.md “Publishing Crates”, which drives version changes via scripts/release-crate.ps1). Please revert the version bump here and do the bump as part of the release PR instead (which will also update Cargo.lock).
version = "0.2.1"
crates/cargo-gamma-process/Cargo.toml:7
- This PR bumps the crate version, but the repo’s documented publishing flow expects version bumps to be done in a dedicated release PR after the desired changes are already on main (see README.md “Publishing Crates”, which drives version changes via scripts/release-crate.ps1). Please revert the version bump here and do the bump as part of the release PR instead (which will also update Cargo.lock).
version = "0.2.1"
crates/cargo-gamma-lib/Cargo.toml:7
- This PR bumps the crate version, but the repo’s documented publishing flow expects version bumps to be done in a dedicated release PR after the desired changes are already on main (see README.md “Publishing Crates”, which drives version changes via scripts/release-crate.ps1). Please revert the version bump here and do the bump as part of the release PR instead (which will also update Cargo.lock).
version = "0.2.1"
crates/cargo-gamma-engine/Cargo.toml:7
- This PR bumps the crate version, but the repo’s documented publishing flow expects version bumps to be done in a dedicated release PR after the desired changes are already on main (see README.md “Publishing Crates”, which drives version changes via scripts/release-crate.ps1). Please revert the version bump here and do the bump as part of the release PR instead (which will also update Cargo.lock).
version = "0.2.1"
crates/cargo-gamma-attrs/Cargo.toml:7
- This PR bumps the crate version, but the repo’s documented publishing flow expects version bumps to be done in a dedicated release PR after the desired changes are already on main (see README.md “Publishing Crates”, which drives version changes via scripts/release-crate.ps1). Please revert the version bump here and do the bump as part of the release PR instead (which will also update Cargo.lock).
version = "0.2.1"
crates/cargo-gamma-attrs-impl/Cargo.toml:7
- This PR bumps the crate version, but the repo’s documented publishing flow expects version bumps to be done in a dedicated release PR after the desired changes are already on main (see README.md “Publishing Crates”, which drives version changes via scripts/release-crate.ps1). Please revert the version bump here and do the bump as part of the release PR instead (which will also update Cargo.lock).
version = "0.2.1"
Cargo.toml:41
- These workspace dependency version bumps should be done as part of the dedicated release PR/workflow (README.md “Publishing Crates” via scripts/release-crate.ps1), not bundled into a feature/fix PR. Please revert these version numbers here; the release PR can update the workspace manifest and Cargo.lock together.
cargo-gamma-attrs-impl = { path = "crates/cargo-gamma-attrs-impl", default-features = false, version = "0.2.1" }
cargo-gamma-engine = { path = "crates/cargo-gamma-engine", default-features = false, version = "0.2.1" }
cargo-gamma-lib = { path = "crates/cargo-gamma-lib", default-features = false, version = "0.2.1" }
cargo-gamma-process = { path = "crates/cargo-gamma-process", default-features = false, version = "0.2.1" }
cargo-gamma-rt = { path = "crates/cargo-gamma-rt", default-features = false, version = "0.2.1" }
Cargo.toml:42
- These workspace dependency version bumps should be done as part of the dedicated release PR/workflow (README.md “Publishing Crates” via scripts/release-crate.ps1), not bundled into a feature/fix PR. Please revert this version number here; the release PR can update the workspace manifest and Cargo.lock together.
cargo-gamma-unsafe = { path = "crates/cargo-gamma-unsafe", default-features = false, version = "0.2.1" }
crates/cargo-gamma-rt/Cargo.toml:7
- This PR bumps the crate version, but the repo’s documented publishing flow expects version bumps to be done in a dedicated release PR after the desired changes are already on main (see README.md “Publishing Crates”, which drives version changes via scripts/release-crate.ps1). Please revert the version bump here and do the bump as part of the release PR instead (which will also update Cargo.lock).
version = "0.2.1"
- Files reviewed: 15/16 changed files
- Comments generated: 0 new
- Review effort level: Lite
a363479 to
1c74f77
Compare
There was a problem hiding this comment.
🟡 Changes recommended
It includes release-version bumps that should be split into the documented release flow, and it persists baseline output tails in JSON without control-encoding.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Review details
Suppressed comments (1)
Cargo.toml:42
- This PR bumps multiple
cargo-gamma*crate versions (and workspace dependency versions) to 0.2.1. Per the repo’s documented release flow, version bumps are supposed to be done via a dedicated release PR generated byscripts/release-crate.ps1(README.md:68-85); mixing them into a behavior/bugfix PR makes it harder to separate release automation concerns from functional changes.
cargo-gamma-attrs-impl = { path = "crates/cargo-gamma-attrs-impl", default-features = false, version = "0.2.1" }
cargo-gamma-engine = { path = "crates/cargo-gamma-engine", default-features = false, version = "0.2.1" }
cargo-gamma-lib = { path = "crates/cargo-gamma-lib", default-features = false, version = "0.2.1" }
cargo-gamma-process = { path = "crates/cargo-gamma-process", default-features = false, version = "0.2.1" }
cargo-gamma-rt = { path = "crates/cargo-gamma-rt", default-features = false, version = "0.2.1" }
cargo-gamma-unsafe = { path = "crates/cargo-gamma-unsafe", default-features = false, version = "0.2.1" }
- Files reviewed: 19/20 changed files
- Comments generated: 1
- Review effort level: Lite
1c74f77 to
ac0d014
Compare
ac0d014 to
9e0e27e
Compare
There was a problem hiding this comment.
🔵 Needs a closer look
It includes crate/workspace version bumps and lockfile updates that should be handled via the documented release process rather than bundled into a fix PR.
Review details
Suppressed comments (9)
crates/cargo-gamma-unsafe/Cargo.toml:7
- This PR bumps the crate version. Per the documented release process, version bumps should be performed in a dedicated release PR via
./scripts/release-crate.ps1(README.md:70-85); please revert this version change in this fix PR.
version = "0.2.1"
crates/cargo-gamma/Cargo.toml:7
- This PR bumps the crate version. The repo release workflow expects version bumps to be done in a dedicated release PR via
./scripts/release-crate.ps1(README.md:70-85), not bundled into a fix PR—please revert the version change here (and keep any release bump separate).
version = "0.2.1"
crates/cargo-gamma-rt/Cargo.toml:7
- This PR bumps the crate version. The repository’s release workflow calls for version bumps to happen in a release PR driven by
./scripts/release-crate.ps1(README.md:70-85); please revert this version change here.
version = "0.2.1"
crates/cargo-gamma-process/Cargo.toml:7
- This PR bumps the crate version. Version bumps are expected to be done in a dedicated release PR (README.md:70-85); please revert this version change in this fix PR.
version = "0.2.1"
crates/cargo-gamma-lib/Cargo.toml:7
- This PR bumps the crate version. The documented release process uses
./scripts/release-crate.ps1in a dedicated release PR to bump versions (README.md:70-85); please revert this version change here.
version = "0.2.1"
crates/cargo-gamma-engine/Cargo.toml:7
- This PR bumps the crate version. Per README.md’s release workflow (README.md:70-85), version bumps should be in a dedicated release PR via
./scripts/release-crate.ps1; please revert this version change in this PR.
version = "0.2.1"
crates/cargo-gamma-attrs/Cargo.toml:7
- This PR bumps the crate version. The repository release process expects version bumps to be handled in a dedicated release PR via
./scripts/release-crate.ps1(README.md:70-85); please revert this version change here.
version = "0.2.1"
crates/cargo-gamma-attrs-impl/Cargo.toml:7
- This PR bumps the crate version. Per the documented release workflow (README.md:70-85), please revert this version bump from the fix PR and do any release bump separately via
./scripts/release-crate.ps1.
version = "0.2.1"
Cargo.toml:37
- These workspace dependency entries are bumped to 0.2.1, but the repo’s documented release process expects version bumps to be done in a dedicated release PR via
./scripts/release-crate.ps1(README.md:70-85). Please revert these workspace version bumps in this fix PR (and regenerate Cargo.lock accordingly).
cargo-gamma-attrs-impl = { path = "crates/cargo-gamma-attrs-impl", default-features = false, version = "0.2.1" }
- Files reviewed: 19/20 changed files
- Comments generated: 0 new
- Review effort level: Lite
9e0e27e to
645845d
Compare
There was a problem hiding this comment.
🔵 Needs a closer look
It changes multiple core execution paths (staged builds, output supervision, and baseline failure artifacting) across many crates, so it needs final human review despite only a small concrete issue found here.
Review details
Suppressed comments (1)
crates/cargo-gamma-lib/docs/DESIGN.md:72
- The design doc says the baseline keeps a “200-line tail”, but the implementation retains
OUTPUT_TAIL_LINES: 2_000(seecrates/cargo-gamma-lib/src/exec/verdict.rs:1644). Update the doc (or the constant) so they match; otherwise readers will mis-size expectations forbaseline-failure.json.
- Files reviewed: 26/27 changed files
- Comments generated: 0 new
- Review effort level: Lite
a1e51c0 to
b326e03
Compare
There was a problem hiding this comment.
🔵 Needs a closer look
The updated design doc’s baseline tail line count doesn’t match the implemented limit (documentation/contract drift).
Review details
Suppressed comments (1)
crates/cargo-gamma-lib/docs/DESIGN.md:72
- The design doc says baseline failures retain a "200-line tail", but the implementation retains up to 2,000 lines (OUTPUT_TAIL_LINES = 2_000 in
crates/cargo-gamma-lib/src/exec/verdict.rs:1644). This is contract drift between the design text and behavior; update the doc (or the constant) so they match.
- Files reviewed: 26/27 changed files
- Comments generated: 0 new
- Review effort level: Lite
b326e03 to
6d6b13b
Compare
There was a problem hiding this comment.
🟡 Changes recommended
There are a few concrete contract/schema inconsistencies and small maintainability issues (notably the baseline tail line-count doc mismatch and artifact string typing) that should be corrected before approval.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Review details
Suppressed comments (1)
crates/cargo-gamma-lib/docs/DESIGN.md:72
- The design doc says baseline failures retain a “200-line” output tail, but the implementation retains 2,000 lines (OUTPUT_TAIL_LINES = 2_000 in exec/verdict.rs). Update the contract text to match the actual bound so readers don’t mis-size expectations.
- Files reviewed: 26/27 changed files
- Comments generated: 2
- Review effort level: Lite
6d6b13b to
7dc2456
Compare
There was a problem hiding this comment.
🔵 Needs a closer look
The updated design documentation claims a 200-line baseline tail, but the implementation retains 2,000 lines, so the contract text needs to be aligned with the code.
Review details
Suppressed comments (1)
crates/cargo-gamma-lib/docs/DESIGN.md:73
- The design doc says the baseline retains a "64 KiB, 200-line tail", but the implementation retains 2,000 lines (OUTPUT_TAIL_LINES = 2_000 in
exec/verdict.rs). Update the doc to match the actual retention limit (or adjust the code if 200 lines was intended).
- Files reviewed: 26/27 changed files
- Comments generated: 0 new
- Review effort level: Lite
martintmk
left a comment
martintmk
left a comment
There was a problem hiding this comment.
[AI AGENT]: I reviewed the changed cargo gamma CLI/artifact contract and traced staged workspace selection, mutant withdrawal, manifest rewriting, baseline supervision, output capture, cleanup, diagnostics, and the changed tests. The coordinated 0.2.1 family bump is internally consistent and explicitly approved as this PR's release vehicle; no supported Rust API change was found. Green CI was used as the baseline, with focused probes for the findings below.
Beyond the five unresolved threads already on the PR, two blocking issues remain: an announced libtest failure can be converted into a timeout, and baseline diagnostics are persisted only after synchronous scratch teardown. Two inline non-blocking findings cover output-tail shifting and failure-artifact documentation.
non-blocking: Moving crates/cargo-gamma-lib/docs/design/README.md to docs/DESIGN.md also broke both reciprocal links: the moved document escapes docs with ../IMPLEMENTATION.md, while docs/IMPLEMENTATION.md still points to the deleted old path. Update them to sibling IMPLEMENTATION.md and DESIGN.md links.
Verdict: changes requested.
7dc2456 to
fd7d831
Compare
There was a problem hiding this comment.
🔵 Needs a closer look
It changes core execution/build orchestration and failure-artifact persistence across multiple crates and layers, warranting final human review despite strong accompanying test/doc updates.
Review details
- Files reviewed: 28/29 changed files
- Comments generated: 0 new
- Review effort level: Lite
- Keep whole-workspace Cargo roots stable across staged convergence while physically withdrawing deferred-stage mutants. This preserves dependency feature unification and prevents unrelated mutants from affecting each stage. - Bound Cargo output safely without reserving the full ceiling, and normalize leading ANSI erase sequences so redirected progress remains readable. - Remove the noisy cgroup-containment warning while retaining best-effort process-group cleanup on hosts without delegated cgroups. - Make baseline failures self-diagnosing. Record termination, timing, memory, package, target, runner, working directory, and bounded control-encoded stdout/stderr tails in baseline-failure.json. - Write baseline-failure.json and gamma-diagnostics.json before early cleanup, and point the console diagnostic at both files instead of replaying captured test output. Artifact failures remain secondary and inherited environment values stay private. - Restrict failure-output retention and complete harness draining to baseline measurement, leaving successful observations and mutation/confirmation hot paths unchanged. - Preserve an existing `cargo-gamma-rt` dependency's `features`/`default-features` when redirecting it to the vendored runtime, for both direct and workspace-inherited dependencies. - Write vendored runtime dependency paths with portable (`/`) separators so the rewritten manifest parses as valid TOML on Windows. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 6a1863fa-92ca-4fd6-9e0c-12ae4ac61826
fd7d831 to
2075242
Compare
Stabilize staged workspace builds. Keep a constant whole-workspace Cargo root set when the resolved selection covers every member, while physically withdrawing deferred-stage mutants and limiting blame to the current stage. This preserves dependency feature unification, prevents repeated dependency variants, and keeps unrelated mutants out of each stage.
Make build-output capture robust. Raise each Cargo stream limit from 4 MiB to 256 MiB and each logical-line limit from 64 KiB to 1 MiB, while growing buffers on demand instead of reserving the ceiling up front.
Normalize Cargo progress rendering. Consume Cargo's leading ANSI erase-to-end-of-line sequence so redirected or non-ANSI displays do not print a literal
\e[K.Remove the containment startup warning. Hosts without delegated cgroups still use the existing best-effort process-group fallback, but cargo-gamma no longer emits the noisy warning.
Make baseline failures self-diagnosing.
baseline-failure.jsonrecords the package, target, runner, executable, working directory, failure classification, termination, elapsed and budget times, memory figures, controlled environment changes, and bounded control-encoded stdout/stderr tails. Successful observations retain no captured output.Persist both
baseline-failure.jsonand the ordinarygamma-diagnostics.jsonbefore early failure cleanup. The console now shows conciseDiagnostics:paths to those files instead of replaying captured test output; artifact-write failures remain secondary to the original baseline error.Keep the mutation hot path unchanged. Full failure-output draining and tail retention apply only to baseline measurement, not mutant execution or confirmation runs.
Bump the cargo-gamma crate family to
0.2.1as a coordinated patch release, including workspace requirements and the lockfile, so the injected runtime crate stays version-aligned.Co-authored-by: Copilot 223556219+Copilot@users.noreply.github.com