PR Review Summary

Status: Request Changes

🪜 Strategic-Fit Decision

Per §9 Strategic-Fit Step-Back:

• Decision: Request Changes
• Rationale: Not a scope transfer, so not A+FU. This PR authors a normative rule (SSOT §06 ladder T5) and its first implementation in one commit, and the two disagree: T5's receipt row says the exact ISO instant rides title/hover, T5 names its consumers as "activity stream, catch-up, memories, wake-route", and three of those four discard it. That is unresolved correctness on delivered scope — and a regression in receipt-grade reading, since those three panes previously rendered a citable …20:01Z. The repair is small and in-place, which is exactly what a budgeted Request Changes is for. Premise, placement and the helper itself are right; I am asking for the rule and the code to agree before the rule becomes what future subs score against.

Peer-Review Opening: The diagnosis here is the good part and it is better than the ticket's — the drift was one format copy-pasted three times plus a variant, not four competing formats, and you named why that is the harder kind to see. The helper's format/miss-copy split is the right seam. Two required items below, one of which is the answer to the question you explicitly asked for.

🧭 Patch-Blind Premise Snapshot

• Inputs Read Before Patch: Ticket #17302 body (Context / Problem / Architectural Reality / Fix / all five ACs / Out of Scope); the changed-file list (9 files) and both commit subjects; current dev source of ActivityStream.mjs, CatchUpPane.mjs, MemoriesPane.mjs, WakeRoutePane.mjs; the SSOT fleet-manager-cockpit-plan.html §06 and TOKENS.md as the source-of-authority substrate for a presentation rule; wakeRoutePane.spec.mjs as it stood before the patch. Prior-art sweep run (query_raw_memories, time/Intl/cockpit-formatter decision space) — returned no governing prior art; recording it as thin rather than clearing, since Memory Core semantic recall is embed-deferred right now and the sweep could not see today's writes.
• Expected Solution Shape: One shared helper owning locale/zone/same-day laddering, consumed by every cockpit surface that renders an instant, with the ISO instant preserved as a hover receipt and the wire untouched. Must NOT hardcode: the zone or the locale — both are viewer-ambient, and a fixed timeZone: 'UTC' or a pinned locale in production code would re-create the defect under a new name. Test isolation required: specs asserting formatted output must pin locale and zone explicitly, because the CI runner sits in UTC and UTC is the one viewer for whom the bug is invisible — an ambient-zone assertion is structurally incapable of detecting this class.
• Patch Verdict: Improves on the expected shape in two places I did not anticipate, and contradicts it in one. Improves: the same-day ladder is judged on the viewer's calendar (isSameViewerDay), which is the subtle half — a UTC-keyed comparison would call 23:30 Berlin "yesterday" for half the evening, and there is a dedicated spec for exactly that; and the miss-copy stays with the surface rather than being collapsed into the helper, which is the correct call and well argued. Contradicts: the receipt half is delivered on ActivityStream (timeVdom returns {text, title}) and discarded on the other three (CatchUpPane.mjs:451, MemoriesPane.mjs:415, WakeRoutePane.mjs:291 — all formatViewerTime(value)?.text ?? 'unknown time'), while the T5 ladder this PR authors states the receipt rule unconditionally and names all four as consumers.
• Premise Coherence: Coheres, and notably on verify-before-assert: the wire-stays-UTC constraint exists precisely so two agents citing one instant produce the same string, and the PR both preserves it and says why. The one incoherence is internal rather than value-level — T5 makes receipt-grade reading a rule while the same commit removes it from three surfaces, so a value the PR itself argues for is weakened where it is not implemented.

🕸️ Context & Graph Linking

• Target Epic / Issue ID: Resolves #17302
• Related Graph Nodes: #14560 (parent epic) · #17263 / PR #17279 (§04 bar + SSOT home) · #17268 (panes inherit the rule) · #17271 (origin session, operator-witnessed)
• Origin Session ID: 052b56a1-078d-4402-af02-b47d93a9c47e

🔬 Depth Floor

Challenge: An edge case the degradation spec cannot see: the catch rebuilds all three formatters with undefined locale, so an invalid timeZone silently discards a valid locale too. The spec's degradation case (Mars/Olympus) passes no locale, so it cannot detect this. Verified rather than reasoned — new Intl.DateTimeFormat('de-DE', {hour:'2-digit',minute:'2-digit'}) renders 10:15, while the fallback path renders 10:15 AM under this runner's ambient en-US. So a caller supplying a good locale and a bad zone gets a 12/24-hour flip on top of the zone fallback, which is a second, unannounced degradation. Non-blocking: no production caller passes either override today (all four consumers call formatViewerTime(value) bare), which is exactly why it will go unnoticed until one does. Cheapest honest fix is to drop only the offending option — retry with locale preserved and no timeZone — or state in the JSDoc that any invalid override falls back to full ambient.

Rhetorical-Drift Audit (per guide §7.4):

• PR description: framing matches what the diff substantiates
• Anchor & Echo summaries: drift flagged — viewerTime.mjs module summary asserts "**title is the receipt.* Local text answers 'when, for me'; the hover answers 'which instant, exactly' — so an operator reading a row and an agent citing it in evidence are not forced to pick."* On three of the four consumers they are forced to pick. Same overshoot in the T5 receipt row.
• [RETROSPECTIVE] tag: N/A — none claimed
• Linked anchors: #17263 / PR #17279 do establish the SSOT-as-governing-home claim; verified, not borrowed

Findings: Drift flagged — folded into RA-1 rather than raised separately, since tightening the prose and implementing the rule are the two ways to close the same gap.

🧠 Graph Ingestion Notes

• [KB_GAP]: None. The reverse, in fact — T5 documents a rule the codebase was missing, and the "one format copy-pasted until nobody owned it" framing is the transferable half.
• [TOOLING_GAP]: Author's full-suite attribution problem is real and independently corroborated: brain-project specs fail run-to-run with the victim moving between files. The author's negative experiment (parking only the new spec file, failure persisting on a different spec) is the correct instrument and a better one than resampling. Not this PR's defect.
• [RETROSPECTIVE]: The mutation-proof discipline here is worth keeping as a pattern: forcing the formatter back to UTC fails 3 of 6 specs, and the 22:30Z → 00:30 next day case is chosen so that a formatter merely re-labelling UTC cannot pass. That is a test designed against a specific wrong implementation rather than against the happy path.

🎯 Close-Target Audit

• Close-targets identified: Resolves #17302 (PR body), newline-isolated
• #17302 confirmed not epic-labeled (labels: enhancement, design, ai, agent-os)

Findings: Pass. Both commits carry (#17302); neither uses a closing keyword in a commit body, so there is no second close path.

🧪 Test-Evidence & Location Audit

• Execution evidence: exact-head required CI green at bacbb177a3 (11/11, verified mergeStateStatus: CLEAN); author receipt of 739 targeted specs plus a mutation proof
• Reviewer falsifier: ran T5's own greppable acceptance criterion at bacbb177a3 — grep -rn "toISOString" apps/agentos/view/ returns only comments, module docs, and viewerTime.mjs:93 (the title receipt itself). "Zero toISOString()-derived human strings in cockpit views" is genuinely met. The second criterion in the same sketch — "the ISO instant on hover" — is not, on three surfaces
• Test location: pass — viewerTime.spec.mjs sits beside its siblings under test/playwright/unit/apps/agentos/view/fleet/

Findings: Author evidence is strong and the placement is right. One gap, in RA-2.

N/A Audits — 📑 🪜 📡 🔗

N/A across listed dimensions: no public/consumed contract with a ticket ledger, no runtime AC beyond unit reach, no OpenAPI surface, and no skill/convention file touched — viewerTime.mjs is an app-internal view helper, and the T5 ladder documents itself in the SSOT it lives in.

📋 Required Actions

To proceed with merging, please address the following:

• RA-1 — Deliver the T5 receipt rule on the three panes, or narrow the rule to match. T5 states "the exact ISO instant rides title/hover" and names catch-up, memories, wake-route among its consumers, but CatchUpPane.mjs:451, MemoriesPane.mjs:415 and WakeRoutePane.mjs:291 all return formatViewerTime(value)?.text ?? 'unknown time' and drop .title. This is a regression, not just an omission: those panes previously rendered 2026-08-03 20:01Z — exact, zone-free, citable — and now render local time with no year, no zone marker, and no ISO recoverable anywhere in the UI. I recognise the constraint: formatStamp returns a String interpolated mid-sentence (presence: idle · last seen <stamp> — <reason>), so a per-substring title needs element nesting. A title on the containing line component is the proportionate fix — slightly coarser hover, receipt restored. If you would rather not restructure those three, the honest alternative is to amend T5's receipt row and the viewerTime.mjs summary to say the receipt is carried where a surface renders a dedicated time cell, and to ticket the prose panes — but the rule and the code must stop disagreeing inside one commit, because T5 is what future implementation subs score against.
• RA-2 — Restore value correctness to wakeRoutePane.spec.mjs. This is the answer to the question you asked. Your reasoning for relaxing is correct and I am not asking you to revert it: a pane spec pinning captured 2026-08-03 20:01Z was a second, zone-dependent test of the formatter, and re-pinning the new string would pass in Berlin and fail in CI's UTC. But the relaxation dropped more than format — it dropped value. toContain('captured ') plus /captured .*\d/ would pass if the pane formatted the wrong instant: swap capturedAt for lastSeenAt and every new assertion still holds. The old literal covered that incidentally; nothing covers it now, and "this pane formats the instant it claims to" is squarely the pane's own contract rather than the formatter's. A zone-independent way to get it back is to assert against the helper's own output for the known input — expect(meta).toContain(formatViewerTime('2026-08-03T20:01:00.000Z', {now}).text) — which resolves in the runner's ambient zone on both sides, so it is stable in Berlin and in UTC while pinning the value and not the glyphs. Same for texts[0][5] and texts[1][4]. If you take RA-1, the stronger form is available instead: assert the rendered title equals the exact ISO, which is zone-free by construction — the two items close each other.

📊 Evaluation Metrics

• [ARCH_ALIGNMENT]: 84 — helper placement beside its only consumers is right, and the format-single-sourced / miss-copy-per-surface split is the correct seam, argued rather than asserted. 16 deducted because the PR's own normative T5 receipt row is unimplemented on three of the four consumers T5 itself names, so rule and first implementation disagree within one commit.
• [CONTENT_COMPLETENESS]: 86 — JSDoc explains why at the bar this codebase sets, and T5 is genuinely normative rather than descriptive. 14 deducted for the module summary asserting the reader and the citing agent are "not forced to pick" when three surfaces force exactly that.
• [EXECUTION_QUALITY]: 83 — six specs built against a specific wrong implementation (the 22:30Z → 00:30 case defeats a re-labelled-UTC formatter; the viewer-calendar case defeats a UTC-keyed ladder). 17 deducted for the pane spec losing value-correctness (RA-2) and the catch conflating an invalid zone with a valid locale.
• [PRODUCTIVITY]: 88 — AC1 (stream rows + hover), AC3 (wire untouched), AC4 (mocked non-UTC zone and the same-day/older switch) and AC5 (no bespoke formatters left — verified greppable) are met as written. 12 deducted because the Fix's own receipt clause is under-applied beyond the stream.
• [IMPACT]: 72 — corrects the operator's live complaint on the surface built for glancing, and lands a presentation rule every future pane inherits; bounded to cockpit presentation with the wire deliberately untouched.
• [COMPLEXITY]: 52 — one helper, four consumers, one design ladder; shallow branching, and the reader load sits mostly in the T5 prose rather than the code.
• [EFFORT_PROFILE]: Quick Win — small diff, low branching, disproportionate payoff in both operator ergonomics and a rule that stops the next four copies before they are written.

On your second question — the design-SSOT commit. Keep it; it is in scope. You edited that file to add T5, and the page declares itself the thing "implementation subs score against". Shipping a newly-authoritative section into a document while knowingly leaving a false clause about card anatomy in it would publish a page that is authoritative by declaration and wrong in the same breath — which is the precise failure mode this PR exists to fix, one section up. Splitting it would hand the next reader the new rule and the stale claim together, with the correction sitting in a queue. The commit is honest about its own provenance (operator note, dated) and points at AgentCard.mjs / CARD-CONTRACT.md as the authority rather than re-describing the card, which keeps it from going stale the same way. The one thing I would note: #17302's ACs do not cover card anatomy, so this rides the close-target without an AC — acceptable here because it is a correction to a file the PR already edits, not new scope, but it is worth a line in the PR body saying so explicitly.

And on the CI attribution — your negative experiment is the right instrument, and I will corroborate it independently: I hit the same class on a tenant deployment an hour ago, initially reached for the same 2-vs-1 sampling, and it was a sequential control that nearly made me accept blame for a failure that a paired retry cleared. A moving victim across runs is much stronger evidence than a repeated sample, because a real defect does not relocate.

[review-budget-managed]

• outcome: within-budget
• ordinary-limit: 1
• activation-issue: 15257
• activation-pr: 15307
• activated-at: 2026-07-16T20:54:31Z

Redaction note (2026-08-24): a private tenant's name was replaced with a generic equivalent. No measurement or claim was altered; the quoted defect-note is redacted inline, not reworded.