[JacobStephens2]

Summary

CommandRecord::finish kept Run metrics only for Run, Spec run and Base fix. So every Review run, Triage run and Fix run saved run_metrics: null, with no Session signals. Weekly trends also skipped Code review session rows.

 CommandRecord::finish
-  if kind in {Run, Spec run, Base fix}
+  if kind in {Run, Spec run, Base fix, Review run, Triage run, Fix run}
     run_metrics = RunMetrics::of(record, usual ranges, sidecars, Session traces)

 StageName::is_session            # weekly trends' Session rows
   implement
+  Code review                    # a Review run's review session
   Resume, Repair, Spec review, …

Usual ranges are unchanged: they are built from every record's stage times and never read run_metrics. Pickup, Architect, Security and Scoring runs, Custom processes and Experiments still keep none (out of scope; Custom processes are #1655).

• Manual: docs/guide.md updated and site/docs/index.html regenerated.
• Hand-written pages: site/index.html and site/how-it-works/index.html corrected. They now name these kinds, say trends split by Command kind, and say a Triage run has no Unaddressed findings (it has no Delivery).

Closes #2047

Evidence

Execution evidence

Confirmed cause: the kind filter in CommandRecord::finish (src/logs/command_record.rs). The regression test drives a Review run, a Triage run and a Fix run through Record::finished with stages, Session traces and a review summary sidecar.

• Before (src/logs/command_record.rs from origin/main): cargo test --bin thirdshift review_triage_and_fix_runs_save

  panicked at src/logs/recording/tests.rs:588:9:
  assertion `left == right` failed: Review run: null
    left: Null
   right: 8.0
  test result: FAILED. 0 passed; 1 failed
  

  After: the same command

  test result: ok. 1 passed; 0 failed
  

  It checks total_minutes, stage and summary session_signals, review_findings from summary.json, and the Fix run's repairs: 2 and ci_rounds: 2.

• Before (without CodeReview in is_session): cargo test --test metrics review_triage_and_fix

  panicked at tests/metrics.rs:785:32:
  no Review run Code review row in { …
  test result: FAILED. 0 passed; 1 failed
  

  After: the same command

  test result: ok. 1 passed; 0 failed
  

  It checks metrics <name> --json readback for each kind, plus the weekly Command-kind and Session rows.

• Final validation: scripts/affected-tests main gave PASS npm test --prefix site (31.82s) and PASS cargo test (894.84s) (full suite). It ran on the first commit; after the review fixes, cargo test --bin thirdshift logs::recording::tests passed (32) and cargo clippy --all-targets was clean.

• Manual check: npm run docs:check --prefix site passed.

Visual evidence

None. The site changes only reword prose inside existing paragraphs, with no markup, CSS or layout change.

Merge Danger

Door: two-way

The change adds a field value to new Command records. Old records are not rewritten, and reverting stops writing it.

Blast Radius: metrics

Only show --json, Console details and thirdshift metrics gain values and rows for these kinds. Ticket run, Spec run and Base fix metrics are unchanged.

Review

The reviewers read every changed file except the generated site/docs/index.html. Both reviewers confirmed excluding it: its source docs/guide.md was reviewed and npm run docs:check --prefix site passed.

Addressed:

• S2: the regression test's expectations now live in a named case table.
• S3: the fake's Session signals are opt-in through observed_sessions.
• S4: the site pages now say weekly trends split by Command kind.
• P1: the site pages no longer imply a Triage run records Unaddressed findings.

Unaddressed findings

Standards

• S1. Primitive Obsession (judgement): finish matches kind strings. Declined. CommandRecord.kind is a serialized String by design (src/logs/command_record.rs:87). This change extends the existing matches!(self.kind.as_str(), …) at that site, and the same string kinds are read across the codebase, for example src/logs/status/inbox.rs:115. A typed kind would be a separate refactor. The new tests cover the three kinds' labels, so a rename that drops metrics would fail them.

Spec

None.

Security

None.

Built with claude 2.1.314 · opus · default effort

Security review outcome

No unaddressed introduced findings.