Created
Jul 07, 22:30
Started
Jul 07, 22:31
Completed
Jul 08, 07:12
DevOps handoff
Type
Feature
Shape
ui+db
Worktree Slug
fix-deployment-duration-metrics
Repositories
mcritchie-studio
Release Slug
—
Branch
feat/fix-deployment-duration-metrics
Acceptance Criteria
Expected Test Plan
Checks Run
Agent Context
Fixes /deployments/all (ReleasesController#index + app/views/releases/index.html.erb) and the /deployments card (app/views/tasks/_release_duration_card.html.erb). Root problems: (1) DEPLOY column is mislabeled — it reads deployment_seconds = created_at->shipped_at, i.e. the TOTAL, not the deploy; (2) the real prod-deploy stamp prod_deploy_started_at->shipped_at is unused; (3) summary cards (BUILDING/REVIEWING/ASSEMBLING/SHIPPING) come from per-task task_events spans while columns come from per-release stamps — two engines, same labels, different numbers; (4) active releases fall back to live_seconds (created->now) inflating DEPLOY (an 8h in-progress next to shipped 15-55m). Operator chose 'release-centric & consistent': cards AND columns both derive from releases.* stamps. New release_metrics: assembly_seconds(created->assembled)=ASSEMBLE, confirmation_seconds(assembled->confirmed)=CONFIRM, deploy_seconds(prod_deploy_started->shipped, fallback confirmed->shipped)=DEPLOY, deployment_seconds(created->shipped)=TOTAL. averages_from rebuilt to average those four release-level fields over recent shipped releases (bump limit 3->10 for stability). Active releases: TOTAL renders live elapsed with a distinct muted 'running' treatment, not a comparable total. Also tighten compact_stage_duration to compound units (1h30m) to stop hour-boundary truncation. Key files: app/models/release/duration_cache.rb, app/views/releases/index.html.erb, app/views/tasks/_release_duration_card.html.erb, app/helpers/application_helper.rb.
How long this task spent in each testing phase — Build → Local Certification → CI → Review → Operator Acceptance.
Build
about 8 hours
Completed
Local Certification
—
Missing
CI
—
Missing
Review
12 minutes
Completed
Operator Acceptance
about 15 hours so far
In progress
Stage Timeline
Who handled each stage, the time it took (measured), and the model / tokens / cost reported (best-effort) — plus who's on it right now. — means the agent didn't report that metric.
Conversation
QA review feedback, agent handoffs, and follow-up notes for this task.
Scout report: merge-ready - Backend light read clean: reversible nullable tested_at migration, best-effort conductor stamp bracket, nil-safe span math, honest legacy-row handling Reporter: carl Outcome: merge-ready Findings: - deployment_span handles all stamp states: legacy shipped rows w/o tested_at read missing/em-dash and drop out of averages via filter_map (per-stage sample_count honest) - Tracker safe: RELEASE_TRACKER_STAGES is name-keyed, tested inserts into STAGES without adding a node; dev advance toy skips it - record_release_event rescues SystemExit+StandardError so a board outage cannot abort prepare; tested not in STAGES_THAT_MAY_OPEN so no ghost RC - Non-blocker: reopen! keeps tested_at (first-write-wins), so a re-prepared RC keeps the first test run's Tested span — consistent with kept testing/assembling starts - Non-blocker: releases/show Stage Averages still reads legacy DurationCache; detail page can disagree with new table until migrated Questions: - none Checks: - gh pr view 453: OPEN, not draft, base release, CI green (test/lint/scans) - e2e Release Steps heading matches current origin/release
Scout report: request-changes - Deep review: model/migration/conductor wiring, live ticker, docs all solid; certs verified at PR tree; one data-quality defect in the new averages chart (nil average renders as <1m with a bar instead of the em-dash the sibling partials use) Reporter: shannon Outcome: request-changes Findings: - BLOCKER _deployment_average_chart.html.erb:25 — stages.dig(key,'average_seconds').to_i coerces nil→0 → label '<1m' + 2%-floor bar for a stage with NO samples; post-deploy every shipped release lacks tested_at so both charts show a false 'Tested <1m' while the pinned average rows/tiles on the same page correctly show '—'. Fix: keep nil, render '—' and skip the bar (add a helper-test case). - MERGE-SEQUENCING: PR #454 inserts helpers at the same application_helper.rb anchor (after compact_stage_duration) and same test file region — whichever merges second conflicts; #454 also carries the OLD release_duration_label body as context, resolve toward #453's precise version - NIT: deployment_clock (this PR) duplicates #454's clock_12h — consolidate post-merge - NIT: Release#deployment_span is public but internal-only; unknown key in deployment_stage_span raises NoMethodError on nil rather than ArgumentError Questions: - none Checks: - gh pr view: OPEN, not draft, base release, CI green (scan_ruby/scan_js/lint/test), MERGEABLE CLEAN - cert tree verified: origin/feat/fix-deployment-duration-metrics^{tree} = e9e1645... = full-suite+rubocop fingerprints (dor-check STALE = primary-checkout false positive) - ticker contract verified: data-prefix honored in default mode; range-script clock matches deployment_clock noon/midnight - no dual render path: DeploymentsBroadcaster only re-renders current_release/last_release/task_card; Last Release empty-state untouched
BLOCKING (shannon primary; carl light approved the model/conductor layer, which is nil-safe - the defect is in the view): app/views/releases/_deployment_average_chart.html.erb:25 - stages.dig(stage[:key], "average_seconds").to_i coerces a nil average to 0, and line 26's 2%-floor draws a colored bar, so the chart renders a false "Tested <1m" for every pre-migration release (all currently-shipped releases have nil Tested spans since tested_at is brand new). The pinned average rows and summary tiles on the same page correctly pass nil through release_duration_label and show an em-dash, so the dashboard contradicts itself on the exact surface this data-quality task exists to fix. FIX: keep nil (drop the .to_i coercion), render the em-dash, skip the bar for nil averages, and add one helper/component test covering the nil-average case. AT RESUBMIT: post bin/task note --resolves-feedback (PR #453 is not draft; no other gate work needed). CARRY AS NON-BLOCKERS: (1) merge-sequencing with PR #454 - both PRs insert helpers at the same application_helper.rb anchor after compact_stage_duration; whichever merges second conflicts, and resolution must keep this PR's precise_stage_duration body; (2) deployment_clock here near-duplicates #454's clock_12h - consolidation follow-up once both land; (3) reopen! keeps the first run's tested_at (consistent with keep-true-origin doctrine, noting only); (4) releases/show Stage Averages still reads legacy DurationCache and can disagree with the new table until migrated. Everything else verified clean: dor-check STALE cert was a confirmed false positive (branch tree equals recorded fingerprint e9e164569040b6c7e7f4fd57119d13ab9c0feba9), migration reversible, conductor wiring correct and rescue-guarded, tracker monotonic-safe, CI fully green. This is a one-line-plus-test rework; resubmit should be quick.
Resolved Shannon's block (chart false <1m): the average chart no longer coerces a nil per-stage average to 0 via .to_i. nil now renders a zero-width bar + em-dash value; the 2%-floor applies only to real non-nil values. A stage with no completed samples (e.g. Tested on pre-migration releases) reads em-dash, not a phantom <1m bar. Added a regression test for the nil-average case. Re-certified green at d62d614a: 3105 runs, 0 failures, rubocop clean, DoR met.
Scout report: merge-ready - LIGHT re-review (Carl): prior blocker fix verified — chart partial keeps nil averages nil (zero-width bar + em-dash, 2%-floor only for real values) with a regression test; model/conductor layer byte-identical since prior approval; cert tree d62d614a matches branch head tree; CI 4/4 green including test lane. Reporter: carl Outcome: merge-ready Findings: - Blocker fix present+tested: _deployment_average_chart.html.erb no longer coerces nil via .to_i; release_duration_label(nil) renders em-dash; regression test asserts width:0% + no false 2% bar - Delta since prior review is exactly 2 files (chart partial + helper test); release.rb, bin/release.rb, migration, controllers untouched — no regression in the approved model/conductor layer - Non-defect verified: partial coerces max_seconds via .to_f internally, so pct math is float division despite integer chart_max at call site - Nit (non-blocking): new ERB comment contains % chars (2%-floor) inside <%# %> body — parses fine (no %> sequence) and suite is green, but house comment rules discourage it Questions: - none Checks: - git diff 5c6d5296..82e107d4 --stat = chart partial + application_helper_test only - branch head tree d62d614a6aa5... == claimed cert fingerprint (tree equality, not stale) - gh pr checks 453: lint, scan_js, scan_ruby, test all pass at head 82e107d4
Scout report: merge-ready - Resubmission fixes the blocker exactly as required: the average chart keeps nil per-stage averages (no .to_i), renders a zero-width bar + em-dash, and floors to 2% only for real values; a component regression test pins the nil path (asserts width:0%, no width:2%, em-dash). Cert fresh by tree equality (branch tree d62d614a == full-suite/rubocop fingerprints); CI fully green at head 82e107d4 including the test lane (4m18s). Deep review of the full diff (model spans, averages, conductor review_tests bracket, table/card/chart views, docs, tests) found no new blockers. Reporter: shannon Outcome: merge-ready Findings: - Blocker RESOLVED: _deployment_average_chart.html.erb keeps nil (average_seconds not coerced), pct=0 for nil so no bar, release_duration_label renders em-dash; regression test 'no-data stage as an em dash with a zero-width bar' asserts the exact failure mode - dor-check STALE from the primary checkout is a false positive (fingerprint = write-tree of cwd); branch tree d62d614a6aa5d96e9c9d1a80f2bd4c54aba80ddd equals both recorded evidence fingerprints — cert valid for the PR code; all other DoR gates pass - NIT (non-blocker): new ERB comment in the chart partial contains a % char ('2%-floor') — violates the house zero-percent-chars-in-ERB-comments rule; harmless here (CI+component tests prove post-comment content renders) but reword to '2 percent floor' - NIT (non-blocker): chart stage labels are text-white overlaid at the bar's left edge; for zero/short bars in LIGHT mode the white 11px label sits on the light surface with only a text-shadow — marginal legibility; consider text-heading when pct is small - PROCESS NIT: resolution note (activity 1237) did not clear the red feedback badge — task API still returns block note 1224 as unresolved_feedback; builder should post/note with --resolves-feedback or Avi clear at gate - Carried non-blockers unchanged (deployment_clock vs #454 clock_12h consolidation; reopen! first tested_at; legacy DurationCache on releases/show; PR #454 same-anchor merge sequencing for Steffon's sweep) Questions: - none Checks: - CI at 82e107d4: lint pass, scan_js pass, scan_ruby pass, test pass (4m18s) - Tree equality: git rev-parse origin/feat/fix-deployment-duration-metrics^{tree} == d62d614a... == [full-suite@]/[rubocop@] fingerprints - Acceptance 5/5 verified: per-stage timestamp+duration cells, tested_at migration+conductor bracket, live count-up, Tested/Assembled/Confirmed/Deployed/Total/Tasks columns, SOP+API+design docs updated
Avi review approved (shannon primary + carl light, both merge-ready; nil-average blocker verified fixed with regression test, CI green at 82e107d4); ready for Steffon's qa-release sweep.
Sealed-bid sizing
Edit →Alex (PM)
—
Avi (PO)
SMALL
Dev
SMALL
Actual
XL
We emailed a one-tap sign-in link to . It expires shortly and can only be used once.
No email? Check spam, or close this and try again.