Skip to content

Keep Run metrics in completed Command records - #1140

Merged
JacobStephens2 merged 1 commit into
issue-981from
issue-1033
Oct 23, 2026
Merged

JacobStephens2 merged 1 commit into
issue-981from
issue-1033

Conversation

@JacobStephens2

@JacobStephens2 JacobStephens2 commented Oct 23, 2026 •

Copy link
Copy Markdown
Owner

Summary

Run completion (owning process)
  read repository Usual ranges, excluding this record
  save outcome, Repair count and CI rounds
  save each stage's seconds, minutes and range position
  atomically write Command record v2
show --json → retained run_metrics

Ticket Runs, Base fixes and Spec runs retain their own metrics. Older records remain readable. Saved comparisons stay fixed as repository history changes.

Closes #1033

Evidence

  • Before: cargo test --test run_metrics failed: the completed record's version was 1, with no metrics.
    After: fake-Harness coverage passes for a Run with a CI-fix Repair, a Spec run and its Ticket Runs, Base fix metrics, older records and saved range comparisons.
  • Before: the recorder test returned too_few_runs despite three historical samples.
    After: cargo test --bin thirdshift logs::recording::tests passes all 30 tests, including exact stage minutes and below/within/above/too-few comparisons.
  • cargo check, cargo fmt --check and cargo clippy --all-targets -- -D warnings pass.
  • /usr/bin/time -p scripts/affected-tests issue-981 passes: all 44 site checks and 2,367 Rust tests; 56 Rust tests skipped. Total wall time: 1,415.47 seconds, including compilation. The selected cargo nextest run compiled in 51.03 seconds and executed in 1,302.442 seconds.
  • CI passes on 8f8dfa1, including Rust, Console and site checks. Release checks also pass.

Test seams

  • CLI commands with fake GitHub and Harness executables, observing retained Command records and show --json.
  • The existing Record lifecycle interface with its file and clock adapter, observing saved completion metrics for exact stage durations and Usual ranges.

Review coverage

Independent thirdshift-code-review Standards and Spec reviews use issue-981 (380e63a) as the fixed point. Each read all 17 changed files, including the generated HTML's changed hunks. No generated-output exclusions or unread changed files remain. Both reports contain zero findings.

Unaddressed findings

  • Standards: None.
  • Spec: None.
  • Security: None. Guidance-mode review of the issue-1033 diff against pinned merge base 380e63a found no introduced vulnerability: the new completion-time history read follows the existing same-principal log layout with symlinks skipped, the record deserializer only restores locally-written fractional values, and show --json exposes only outcome, stage times and range positions. No proof-of-concept demonstrated a cross-boundary effect, so no fix was committed.

Security review outcome

No unaddressed introduced findings.

Merge Danger

Door: two-way

The code can be reverted. Version 2 records retain their logs; older binaries skip those records and can recover historical log views.

Blast Radius: local

Command recording and CLI/Console readers on this machine. Completion reads repository history; a history-read failure warns and saves metrics without a Usual range.

Built with codex · gpt-6.1-sol · xhigh

@JacobStephens2
JacobStephens2 merged commit 0767f7c into issue-981 Oct 23, 2026
18 checks passed
@JacobStephens2
JacobStephens2 deleted the issue-1033 branch October 23, 2026 11:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant