Repository navigation
Keep Run metrics in completed Command records - #1140
Merged
Merged
Conversation
This was referenced Oct 23, 2026
Merged
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
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
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
cargo test --test run_metricsfailed: 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.
too_few_runsdespite three historical samples.After:
cargo test --bin thirdshift logs::recording::testspasses all 30 tests, including exact stage minutes and below/within/above/too-few comparisons.cargo check,cargo fmt --checkandcargo clippy --all-targets -- -D warningspass./usr/bin/time -p scripts/affected-tests issue-981passes: all 44 site checks and 2,367 Rust tests; 56 Rust tests skipped. Total wall time: 1,415.47 seconds, including compilation. The selectedcargo nextest runcompiled in 51.03 seconds and executed in 1,302.442 seconds.8f8dfa1, including Rust, Console and site checks. Release checks also pass.Test seams
show --json.Recordlifecycle interface with its file and clock adapter, observing saved completion metrics for exact stage durations and Usual ranges.Review coverage
Independent
thirdshift-code-reviewStandards and Spec reviews useissue-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
380e63afound 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, andshow --jsonexposes 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