[JacobStephens2]

Summary

/metrics was an unlinked <pre> dump of the weekly JSON. It is now a first-class Console page named Metrics (g then m). The page, thirdshift metrics --json and /api/v1/metrics all read one shared document.

 thirdshift metrics [<command>]            # one Command: unchanged
+  [--window 7d|30d|12w|all]               # presets ending today (UTC)
+  [--grain day|week] [--from] [--through] # explicit buckets; Mondays by week
+  [--repo] [--kind] [--harness] [--model] [--account]   # repeatable
 {
   version, hostname,
+  window: {preset, grain, from, through, prior},
+  filters, unavailable_accounts,
+  outcomes: [{bucket, account, kind, succeeded, failed, paused, stopped, live,
+              n, observed, missing, success_rate, sub_outcomes}],
+  comparison: {current, prior, rows: [{account, kind, current, prior, delta}]} | null,
-  weeks: [...]
+  weeks | days: [...]          # signal rows, now tagged by account
 }

How each Command is classified:

classify(command)
  skipped Pass, or Pickup run that dispatched work  -> not counted (its dispatch counts)
  kind not a work Command or Pickup run             -> not counted
  live                         -> Live (missing; outside the rate)
  died                         -> Failed  (sub-outcome "died")
  paused                       -> Paused
  interrupted + Stop request (own or owner's) -> Stopped
  *failed*, review required, other interrupted -> Failed
  anything else                -> Succeeded
  Pickup run counts only when Failed (it failed or died before dispatch)
success_rate = Succeeded / (Succeeded + Failed), per OS account and Command kind

src/logs/metrics/
├── scope.rs      # Window (presets, buckets, preceding window) and Filters
├── outcomes.rs   # the four stacks, Live, comparison and delta
└── trends.rs     # signal rows: shared window/filters; Died apart from Live
src/logs/reading.rs        # Snapshot::metrics_for_user: Other-account Commands
src/console/metrics.{rs,html}  # GET filter form, tables, static SVG bars
src/console/nav.html       # Metrics, data-jump="m"

The bars are inline SVG <rect>s coloured only with Theme tokens (--ready, --urgent, --look, --dim). Each segment carries its exact sub-outcomes in a hover <title>, and its counts are also in the table. There is no JavaScript and no motion, and each table scrolls inside its own region, so the page never overflows at 390px. Signal rows now leave out pass-through Pickup runs and skipped Passes, and count a crashed record as died rather than in missing_outcomes.

Evidence

Execution evidence

The two Spec review findings came with tests, which are kept:

• Before (bb4ab807):

  $ cargo test --test metrics signal_rows_keep_died
  test signal_rows_keep_died_apart_from_live_and_drop_pass_through_pickups ... FAILED
  $ cargo test --test console metrics_form_keeps
  test metrics::metrics_form_keeps_every_repeated_filter_value ... FAILED
  

  After (945f5f09): both ... ok.

• New coverage, all passing:

  • tests/metrics.rs:
    • outcomes_stack_by_kind_without_pass_through_pickups_or_skipped_passes
    • bounded_windows_compare_with_the_preceding_window
    • kind_harness_and_model_filters_narrow_outcomes_and_signals
  • tests/console/metrics.rs: metrics_page_shows_outcome_stacks_and_tables_and_keeps_single_command_inspection
  • tests/console/other_accounts.rs: other_account_commands_count_in_metrics_tagged_by_account
  • tests/console/api.rs: API equals CLI for window 7d/30d/12w/all, grain/from/through, repo, kind, harness, model and account.
  • Unit tests in logs::metrics::{scope,outcomes}.
  • console/tests/metrics.test.cjs:
    • keyboard g m
    • 12 built-in Themes plus a custom Theme give four distinct stack fills
    • no animation or transition
    • scrollWidth <= innerWidth at 390px

• scripts/affected-tests origin/issue-2036:

  • npm test --prefix site: PASS
  • cargo build --locked: PASS
  • npm test --prefix console: PASS
  • cargo test: FAIL. On each full run, one or two different tests in tests/console/idle.rs or tests/process/findings.rs failed. Each passes when re-run alone, and a different one fails on each full run. The same flakiness is on the base branch: three runs of cargo test --test process on origin/issue-2036 each failed one or two findings:: tests (fixing_dispatches_each_reproduced_finding_and_keeps_unreproduced_findings_private, closing_rechecks_each_merged_base_and_leaves_a_failing_finding_for_the_day_shift, merged_fix_is_rechecked_after_a_later_fix_fails). After that full run, the affected suites passed when run on their own: cargo test --test console idle::, --test metrics, --test run_metrics, --test scoring, --test console -- metrics other_accounts api:: and --bin thirdshift metrics.

Visual evidence

/metrics, Focus light Theme, same fixture records.

Desktop 1440×900:

Mobile 390×844:

The After captures were taken before the review fix commit. Since then the form shows one field per repeated filter value, which looks the same when no filter repeats.

Merge Danger

Door: two-way

This is a read-only view: it writes no records and adds no stored fields.

Blast Radius: API-consumers

Changes a metrics --json / /api/v1/metrics consumer will see:

• With --grain day, signal rows appear under days.
• Rows gain account.
• A crashed record moves from missing_outcomes to outcomes.died.
• Pass-through Pickup runs and skipped Passes no longer have signal rows.
• --from/--through reject a non-Monday by week, as before, and accept any date with --grain day.

Review

Reports: Standards found 12, Spec found 2.

• Fixed:
  • Standards 1, 2, 11 and 12.
  • Spec 1 and 2, each test run as written: both failed, and both now pass.
• Changed files the review left unread:
  • docs/code-navigation.md: changed by the fix commit after both reviews.
  • site/docs/index.html: generated from docs/guide.md; both reviewers excluded it after npm run docs:check --prefix site exited 0.

Unaddressed findings

None of these needs a Spec or Day shift decision, so no issue was filed.

Standards

All of these are baseline smells (judgement calls). None says behaviour is wrong.

• 3. Possible Duplicated Code (failure classification shape shared with peek.rs:297 and inbox.rs:234).
  • The three sites classify differently on purpose.
  • Metrics: interrupted is Failed unless a Stop was asked (src/logs/metrics/outcomes.rs classify), per Q2.4's human-Stop-only Stopped.
  • The Inbox: interrupted is never Failed.
  • Peek: interrupted is always unsuccessful.
  • Sharing one predicate would change Inbox or peek behaviour, which is outside this ticket.
• 4. Possible Duplicated Code (record walk in outcomes::visit and trends::collect).
  • The walks differ: visit carries the inherited Stop down to children; collect groups by session.
  • The shared part is three lines.
• 5. Possible Data Clumps (filter vectors).
  • Filters is the bundle the core uses.
  • MetricsArgs (parse), Form (one field per value) and the OpenAPI loop each need the five names in their own shape.
• 6. Possible Repeated Switches on Class.
  • name/css/count are total, exhaustive matches; the compiler flags any new variant in each.
  • css() matching serde's names keeps the CSS hook independent of the JSON spelling.
• 7. Possible Primitive Obsession (window and grain names).
  • grain/preset are the serialized forms of unit/MetricsWindow, so the JSON shows names rather than enum shapes.
  • The OpenAPI literals mirror the code but are not derived from it. Low value to change in this PR.
• 8. Possible Repeated Switches (weeks or days).
  • The JSON key must change with the grain to keep weeks backward compatible.
  • Both sites are the boundary where that choice is made.
• 9. Possible Duplicated Code (hostname()).
  • It matches an expression already repeated in five places.
  • Consolidating them touches unrelated modules.
• 10. Possible Divergent Change (presentation helpers in logs::metrics).
  • Tally::rate, Delta::rate and segments() are pure functions of the counts.
  • They are unit-tested beside them (outcomes::tests).
  • Moving them would expose Tally internals to the Console.

Spec

None.

Closes #2065

Built with claude 2.1.314 · opus · default effort

Security review outcome

No unaddressed introduced findings.