Closes #17087.

dev carried the same guard twice — the shared helpers/resolveRowTimestamp.mjs module, and SummaryService's byte-equivalent static resolveSummaryTimestamp. Consolidated onto the shared module.

Changes

1. SummaryService.mjs imports resolveRowTimestamp and calls it at both projection sites (:361, :527).
2. The static and its JSDoc block are deleted; both @returns {@link} references now point at the shared helper.
3. The spec's unit-level arm targets the shared helper. Its assertions are unchanged.

Direction is toward the shared module, per the avoided trap: MemoryService and conceptWalkMemoryGate already consume it, so collapsing the other way would have created importers of a service singleton's static.

On the "delete one copy without moving the JSDoc" trap

I compared the two JSDoc blocks line by line. The shared module's prose already carried both load-bearing contracts — absent-vs-unparseable collapsing to one null, and new Date(null) being epoch 0 so a null-valued timestamp projects as 1970 rather than counting as unprojectable.

The only detail unique to the deleted copy was the concrete escalation it produced. So rather than dropping it, I folded one clause into the shared JSDoc:

A per-record data condition becomes a per-call outage — on the summaries surface that surfaced as a whole-call SUMMARY_QUERY_ERROR.

That keeps the named failure mode discoverable from the surviving helper, which is the part that stops the next person "fixing" the 1970 behaviour.

Behaviour is unchanged — measured, not asserted

I ran the deleted static's exact body side by side with the shared helper across the spec's own UNPROJECTABLE_TIMESTAMPS fixture set plus the epoch-0 parity cases:

OK   {"timestamp":1700000000000}       -> 2023-11-14T22:13:20.000Z | 2023-11-14T22:13:20.000Z
OK   {"timestamp":"not-a-date"}        -> null                     | null
OK   {"timestamp":null}                -> 1970-01-01T00:00:00.000Z | 1970-01-01T00:00:00.000Z
OK   {"timestamp":0}                   -> 1970-01-01T00:00:00.000Z | 1970-01-01T00:00:00.000Z
...
IDENTICAL on all 12 cases

Compared with Object.is, so null vs undefined would have shown as a difference.

Acceptance criteria

• SummaryService has no local timestamp-resolution method; both projections call the shared helper (grep -c "resolveRowTimestamp(metadata)" → 2)
• grep -rn "resolveSummaryTimestamp" ai/ test/ → 0 hits
• Only the direct-helper-reference arm of the spec was retargeted; no behavioural assertion changed
• The null → epoch-0 parity and absent-vs-unparseable contract remain documented on the surviving helper

What I could not run

The two TimestampGuard specs did not execute here. They live in the unit-brain* projects, which Playwright skips unless the Brain tier is installed:

[playwright.config.unit] Brain-tier set not installed (see package.brain.json) — skipping chroma-setup + unit-brain* projects.
Error: No tests found.

npm run install-brain needs better-sqlite3 (native build; no MSVC toolchain on this machine) and chromadb, so I couldn't arm it. CI has to confirm those two specs — flagging it rather than implying green.

What I did verify locally: npm install + npm run bundle-parse5 (needed before the unit config resolves at all), node --check on all three touched files, the equivalence run above, and the full lint-staged pre-commit gate — which, usefully, caught a ticket reference I'd left in a durable comment and which I've rewritten as behaviour prose.

🤖 Generated with Claude Code [QUARANTINED_URL: claude.com]

Pull request overview

This PR removes the duplicated summary-row timestamp guard in SummaryService by consolidating summary projections onto the shared resolveRowTimestamp helper, reducing drift risk while keeping behavior unchanged.

Changes:

• SummaryService now imports and uses resolveRowTimestamp at both summary result projection sites.
• Deleted SummaryService.resolveSummaryTimestamp and repointed JSDoc references to the shared helper.
• Updated the SummaryService timestamp-guard unit spec’s direct-helper arm to target resolveRowTimestamp.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.

     *   A row whose stored `timestamp` is absent or unparseable is returned with `timestamp: null` and
     *   counted in `malformedTimestamps` rather than failing the call — see
     *   {@link module:ai/services/memory-core/helpers/resolveRowTimestamp~resolveRowTimestamp}. `malformedTimestamps` is omitted when zero.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

@neo-opus-ada (COMMENTED) reviewed on 2026-08-14T06:32:10Z

Status: Comment — CI deferral (the blocker is not yours)

🪜 Strategic-Fit Decision

Per §9 Strategic-Fit Step-Back:

• Decision: Approve on merit, deferred on CI
• Rationale: Every acceptance criterion is delivered and I verified each one directly. The only thing standing between this and a formal approval is that no CI has run on the branch at all — fork PRs need a maintainer to authorize workflows, which is not something the contributor can do. Our protocol requires green current-head CI before a formal approval, so this is a deferral rather than a withholding.

Peer-Review Opening: Welcome, and thank you — this is a genuinely well-executed first contribution. I wrote #17087, so I get to say plainly that you delivered exactly what it asked for, plus one thing it only hinted at. Details below; the single outstanding item is a maintainer action, not yours.

🧭 Patch-Blind Premise Snapshot

• Inputs Read Before Patch: Ticket #17087 (its four ACs, Out of Scope, and two Avoided Traps); current origin/dev source of SummaryService.mjs and helpers/resolveRowTimestamp.mjs; the two existing TimestampGuard specs; and the repo's existing {@link module:...} usage to check whether the doc-reference form used here is idiomatic.
• Expected Solution Shape: Import the shared helper, switch both projection call sites, delete the service-local static, and retarget the two {@link} references plus the spec's unit-level arm. The boundary it must not cross is direction: collapsing toward SummaryService instead of toward the shared module would create two importers of a service singleton's static, which the ticket names as its first Avoided Trap. Behaviour must be byte-identical — this is a consolidation, not a semantics change — and the load-bearing JSDoc must survive the deletion rather than vanishing with the copy that carried it.
• Patch Verdict: Matches, and improves on the ask in one place. Both call sites switched (SummaryService.mjs:330, :496), the static and its JSDoc removed, both @returns {@link} references retargeted, and the spec's unit arm now exercises the shared helper with a comment explaining why it points there. The improvement: rather than merely preserving the deleted JSDoc, the summaries-specific detail was folded into the shared module — so the helper now carries the context both callers need instead of losing the half that lived only in the service copy.
• Premise Coherence: Coheres — this closes debt I created deliberately and recorded publicly: #17077 and #17083 both landed a guard, and amending an already-approved PR to share it would have invalidated that approval. The constraint expired when both merged, and leaving two implementations of one guard is the shape that let the original defect exist on two surfaces at once.

🕸️ Context & Graph Linking

• Target Epic / Issue ID: Resolves #17087
• Related Graph Nodes: #17076 / PR #17077 (the summaries-surface guard) · #17082 / PR #17083 (the memory-surface guard that introduced the shared module) · #12628 (the silent-under-retrieval failure class the preserved JSDoc protects against)
• Origin Session ID: 4ad778d4-bdc6-44cc-b6ec-7ef2c9e7af03

🔬 Depth Floor

Challenge OR documented search (per guide §7.1):

• Documented search: I actively looked for three specific failure modes and found none. (1) Wrong-direction consolidation — the ticket's first Avoided Trap was collapsing toward SummaryService, which would leave MemoryService and conceptWalkMemoryGate importing a service singleton's static; the diff moves the other way, so that trap is cleared structurally. (2) Silent contract loss — the second trap was deleting a copy without carrying its JSDoc, since the absent-vs-unparseable rule and the new Date(null) → epoch-0 parity are what stop a future reader from "correcting" the 1970 behaviour; both survive, and the summaries-specific SUMMARY_QUERY_ERROR detail was merged into the shared module rather than dropped. (3) Non-idiomatic doc form — {@link module:...~resolveRowTimestamp} has existing precedent in this repo (hostEdge.mjs, recordTurnPresenceOverMcp.mjs, backup.mjs), so it will not read as invented to the next maintainer. The import also sits in the correct alphabetical position for that file's existing ordering.

Rhetorical-Drift Audit (per guide §7.4):

• PR description matches the diff; no overclaim
• The added JSDoc sentence describes the mechanical reality (a whole-call SUMMARY_QUERY_ERROR) rather than restating the change
• [RETROSPECTIVE] tag: N/A — none carried
• Linked anchors: Closes #17087 resolves to the ticket this delivers

Findings: Pass.

🧠 Graph Ingestion Notes

• [KB_GAP]: None.
• [TOOLING_GAP]: Fork PRs report "no checks reported on the branch" until a maintainer authorizes the workflow run, so a first-time external contribution cannot self-demonstrate CI health. That is working as designed for security, but it means the reviewer-facing evidence gate and the contributor's ability to satisfy it are held by different people — worth knowing when triaging community PRs, since the delay reads as reviewer silence from the contributor's side.
• [RETROSPECTIVE]: A consolidation is judged by what survives the deletion, not by what the deletion removes. The mechanical half here — two call sites and a removed static — is trivially verifiable; the part that determines whether the change is an improvement or a slow loss is whether the reasoning attached to the deleted copy makes it into the surviving one. Folding the caller-specific detail into the shared module, rather than preserving the shared text and discarding the specific, is the version that leaves the codebase knowing more than before.

N/A Audits — 📑 📡 🔗 🪜

N/A across listed dimensions: no consumed contract changes (the helper's signature and behaviour are unchanged), no OpenAPI surface, no cross-substrate convention, and the ACs are static-structure properties with no runtime evidence class beyond the existing specs.

🎯 Close-Target Audit

• Close-targets identified: #17087 via Closes #17087.
• Confirmed not epic-labeled — #17087 carries bug, ai, good first issue, refactoring, agent-os

Findings: Pass with one convention note. Agent-authored PRs in this repo are required to use Resolves #N rather than Closes #N; the lint that enforces it is scoped to agent PRs and may well not fire here, and both keywords close the issue identically on GitHub. If CI comes back complaining about the body, changing Closes #17087. to Resolves #17087 is the whole fix. If it does not complain, leave it — this is not worth a round trip when the code is right.

🧪 Test-Evidence & Location Audit

• Execution evidence: absent, and not the contributor's doing — gh pr checks 17095 reports "no checks reported on the branch". Fork PRs require maintainer workflow authorization.
• Reviewer falsifier: run — git grep -n "resolveSummaryTimestamp" across ai/ and test/ on the PR head returns zero hits, which is AC-2 verified mechanically rather than by inspection.
• Test location: pass — the existing spec is edited in place; no new or relocated test file.

Findings: Pass on substance, deferred on execution. Both TimestampGuard specs retain their behavioural assertions unchanged, which is AC-3's real requirement: only the direct-helper-reference arm was retargeted, and the arms that prove the guard's behaviour were not touched.

📋 Required Actions

None on the code.

• Maintainer action, not the contributor's: authorize the CI workflow run on this fork PR so the evidence gate can be satisfied. @tobiu — once that lands green I will convert this to a formal approval without needing anything further from either of you.

📊 Evaluation Metrics

• [ARCH_ALIGNMENT]: 100 — Consolidates toward the shared module, the correct direction, and does not walk into the ticket's named trap of collapsing toward the service.
• [CONTENT_COMPLETENESS]: 100 — Both {@link} references updated, the load-bearing JSDoc preserved and enriched with the caller-specific detail, and the spec comment explains the retarget for the next reader.
• [EXECUTION_QUALITY]: 95 — Behaviour-identical by construction; the withheld 5 reflects only that CI has not yet demonstrated it, not any finding of mine.
• [PRODUCTIVITY]: 100 — All four ACs delivered.
• [IMPACT]: 40 — Removes a duplicate-implementation drift risk on the Memory Core read path. Modest by design, which is what made it a good first issue.
• [COMPLEXITY]: 15 — Two call sites, one deletion, two doc references, one spec arm.
• [EFFORT_PROFILE]: Quick Win.

For context on why this was worth ticketing rather than leaving: the guard you consolidated exists because a single malformed row could fail an entire query_summaries call, and the same defect turned out to be sitting on the memory surfaces too — the fallback path everyone relies on when summaries is down. Two implementations of that guard is exactly how such a defect gets fixed in one place and left standing in the other. You closed that. Nice work, and I hope you pick up another.

— Ada (@neo-opus-ada) ⚖️

@neo-opus-ada (APPROVED) reviewed on 2026-08-14T07:05:34Z

PR Review Follow-Up Summary

Status: Approved

Cycle: Cycle 2 follow-up — converting the CI deferral

Opening: Cycle 1 approved this on merit and deferred only because no CI had run on the fork branch; CI has now run green at the same tree I reviewed, so the deferral is discharged.

🧭 Patch-Blind Premise Snapshot

• Inputs Read Before Patch: My Cycle-1 review anchor; the current head SHA against the one I verified; the full check list at that head; and the branch commit log, to confirm nothing was pushed between the two reviews.
• Expected Solution Shape: No code delta was expected or requested — the only outstanding item was execution evidence, which is a maintainer-authorized workflow run rather than anything the contributor could produce.
• Patch Verdict: Unchanged and confirmed. The head is 3c6585772b, a single commit dated 06:23:23Z, and git diff between the tree I reviewed at Cycle 1 and the current head is empty. So the CI result attaches to exactly the code I verified — not a re-push that happened to go green.
• Premise Coherence: Coheres — the deferral existed to satisfy the evidence gate, and the gate is now satisfied against the same tree rather than a moved one.

🪜 Strategic-Fit Decision

Per §9 Strategic-Fit Step-Back:

• Decision: Approve
• Rationale: Every AC was verified at Cycle 1, the only blocker was execution evidence, and that evidence is now green at the identical tree. Nothing remains.

⚓ Prior Review Anchor

• PR: #17095
• Target Issue: #17087
• Prior Review Comment ID: https://github.com/neomjs/neo/pull/17095#pullrequestreview-4934578848
• Author Response Comment ID: N/A — no author action was required
• Latest Head SHA: 3c6585772b
• Origin Session ID: 4ad778d4-bdc6-44cc-b6ec-7ef2c9e7af03

🔁 Delta Scope

• Files changed: none since Cycle 1 — git diff between the reviewed tree and the current head is empty.
• PR body / close-target changes: unchanged.
• Branch freshness / merge state: clean — mergeStateStatus: CLEAN at 3c6585772b.

✅ Previous Required Actions Audit

• Addressed: the CI authorization. This was recorded as a maintainer action rather than a contributor one. It has been performed, and the run completed green.

There were no code-level Required Actions at Cycle 1, and none arise now.

🔬 Delta Depth Floor

• Documented delta search: I checked the three things that make a "CI went green" conversion untrustworthy. (1) Tree identity — the head is the same SHA I reviewed, and the branch carries a single commit, so no silent re-push moved the code under a stale approval. (2) Check completeness — 20 of 20 report pass with an empty non-pass set, rather than a subset passing while others were skipped on a fork. (3) Merge state — CLEAN, so the green is against a mergeable tree rather than one that would need a rebase to land. No new concerns.

N/A Audits — 📑 📡 🔗 🪜

N/A across listed dimensions: no code delta since Cycle 1, so no audit surface changed.

🧪 Test-Evidence & Location Audit

• Evidence: exact-head CI green at 3c6585772b — 20/20 checks pass, non-pass set empty, mergeStateStatus: CLEAN. Cycle 1's reviewer falsifier (git grep -n "resolveSummaryTimestamp" returning zero hits across ai/ and test/) stands unchanged, since the tree did not move.
• Test location: unchanged from prior review — the existing spec edited in place.
• Findings: Pass.

📊 Metrics Delta

• [EXECUTION_QUALITY]: 95 -> 100 — the withheld 5 at Cycle 1 was solely "CI has not demonstrated it yet". It has now, at the same tree.
• [ARCH_ALIGNMENT]: unchanged from prior review (100)
• [CONTENT_COMPLETENESS]: unchanged from prior review (100)
• [PRODUCTIVITY]: unchanged from prior review (100)
• [IMPACT]: 40 -> 65 — correcting my own Cycle-1 score rather than the contributor's work. I scored it as a modest cleanup because no live defect existed. That undersold it: this guard is what stops one malformed row from failing an entire query_summaries or query_raw_memories call, and two copies existed precisely because the same defect had to be fixed twice on two surfaces. Leaving both is how the next fix lands in one and not the other. It belongs inside the Agent OS reliability epic, not beside it.
• [COMPLEXITY]: unchanged from prior review (15)
• [EFFORT_PROFILE]: unchanged from prior review — Quick Win

📋 Required Actions

No required actions — eligible for human merge.

[merge-readiness-uncertified][no-positive-observation] — checks read green at 3c6585772b (observed 2026-08-14T07:03Z); B-prime certification is unavailable in my session because Memory Core identity is unbound. Eligibility is not authorization — the merge is @tobiu's.

@dchaudhari7177 — this is approved and sitting at the human merge gate; nothing further is needed from you. Two notes worth having.

I could not assign #17087 to you: GitHub returns 404 for non-collaborators on the assignee check and silently drops the request, so I recorded your authorship in a comment on the ticket instead. That is a permissions limit, not a judgement about the work.

And a correction I owe you: I scored the impact of this ticket as modest when I filed it and again in my first review, treating it as tidy-up because nothing was visibly broken. That was wrong, and I have raised it above. The duplication you removed is the mechanism by which a fix lands on one surface and quietly misses its twin — which is exactly what happened here in the first place, and is why there were two copies for you to consolidate. It is a good first contribution on a real reliability seam, not a cosmetic one.

— Ada (@neo-opus-ada) ⚖️

@neo-opus-grace (APPROVED) reviewed on 2026-08-14T07:37:35Z

PR Review Summary

Status: Approved

🪜 Strategic-Fit Decision

Per §9 Strategic-Fit Step-Back:

• Decision: Approve
• Rationale: All four acceptance criteria are met and I verified each one independently rather than reading your checklist. Behaviour is byte-identical, the consolidation collapses in the direction the ticket's avoided-trap prescribes, and the JSDoc contract that the ticket calls load-bearing survives — enriched, not merely preserved. No correctness, safety, or code-shape defect. The one item below is a house-convention polish on the PR body, not a code change, and it does not warrant a return cycle.

Peer-Review Opening: Welcome, and thank you — this is a notably strong first contribution. The part I want to single out is that you did not just claim the behaviour was unchanged; you ran the deleted static's exact body against the shared helper across twelve cases and compared with Object.is, so null versus undefined would have shown. That is a stronger form of evidence than most changes of this size arrive with. You also read the ticket's "Avoided Traps" section and addressed the JSDoc trap explicitly instead of discovering it in review. Approving.

🧭 Patch-Blind Premise Snapshot

• Inputs Read Before Patch: #17087's body (Ada's, labelled good first issue, with four ACs and its two Avoided Traps); the changed-file list; the current dev source of SummaryService.mjs, resolveRowTimestamp.mjs, and the third consumer conceptWalkMemoryGate.mjs; test/playwright/playwright.config.unit.mjs for the Brain-tier project gating; ADR-0019 (no config surface touched here, so it is not a gate for this diff).
• Expected Solution Shape: A pure consolidation — import the existing shared helper at both SummaryService projection sites, delete the byte-equivalent static, retarget the two {@link} references and the spec's unit-level arm, and change no behaviour. The direction must be toward the shared module, since MemoryService and conceptWalkMemoryGate already consume it. What this must NOT do: drop the epoch-0 / absent-vs-unparseable prose (Avoided Trap #2), alter the null → 1970 semantics, or touch the behavioural assertions in either TimestampGuard spec.
• Patch Verdict: Matches, and one piece improves on the expected shape. I checked Avoided Trap #2 first because it is the one a consolidation most easily trips: the surviving helper already carried both load-bearing contracts verbatim, so nothing was lost. What you added on top is the part I did not expect — folding the deleted copy's one unique detail (the concrete SUMMARY_QUERY_ERROR escalation) into the shared module's prose, so the named failure mode stays discoverable from the surviving helper. That is the trap's actual intent rather than its letter.
• Premise Coherence: Coheres with friction→gold. #17087 exists because one guard living in two places is the exact shape that let the original defect exist twice; closing that shape inside the repair for it is the substrate lesson landing rather than being noted. Scoped as a pure consolidation with no semantics change, which is what keeps it verifiable.

🕸️ Context & Graph Linking

• Target Epic / Issue ID: Closes #17087 (sub of Epic #17072) — see the Close-Target Audit for the house-convention note
• Related Graph Nodes: #17072 (parent epic), #17076 / PR #17077 (the summaries-surface guard), #17082 / PR #17083 (the memory-surface guard that introduced the shared module and recorded the duplication as deliberate)
• Origin Session ID: 471d17f2-777c-4676-a137-fa37a9ac834d

🔬 Depth Floor

Your open question, answered — the two specs DID run in CI. You flagged that SummaryService.TimestampGuard.spec.mjs and its Memory sibling did not execute locally and asked CI to confirm rather than implying green. That was the right call, and here is the confirmation with its evidence:

1. The spec path contains /ai/ and ends .spec.mjs, so it matches brainTestMatch (/[\\/]ai[\\/].*\.spec\.mjs$/) and is collected by the unit-brain project.
2. The config carries a fail-closed CI guard, assertBrainTierForEnvironment({brainPresent, isCI}), which throws when isCI && !brainPresent, with the reasoning stated in its own message: "a skipped brain matrix on a green CI run is silent coverage loss."

So a green CI run on this repo cannot mean "the brain specs were skipped" — that state fails the run instead. CI is 20/20 pass at 3c6585772b, therefore those specs executed and passed. Your local skip was the designed Body-default install behaviour, not a misconfiguration on your side.

Challenge — a follow-up concern this PR surfaces but correctly does not resolve. With all three consumers now behind one helper, an asymmetry becomes visible: the helper's module JSDoc states the caller contract as "Callers preserve the row with a null timestamp and count it, never dropping it." MemoryService and SummaryService both count (malformedTimestamps++). The third consumer, conceptWalkMemoryGate.mjs:114, projects timestamp: resolveRowTimestamp(metadata) inline and never counts. Nothing is dropped, so there is no under-retrieval — but the observability half of the documented contract is honoured by two of three callers, which means a malformed row reaching the concept-walk path is invisible.

This is pre-existing (it arrived with PR #17083), it is explicitly outside #17087's scope, and it is not something you should fix here — a consolidation that quietly changed a third consumer's output shape would be worse than the asymmetry. I am recording it because the consolidation is what made it legible, which is a point in the change's favour. Worth a small follow-up ticket by a maintainer; I would not ask a first-time contributor to take it as a condition of this merge.

Two minor observations, neither actionable:

• The {@link module:ai/services/memory-core/helpers/resolveRowTimestamp~resolveRowTimestamp} form is idiomatic here — I checked, {@link module:…} targets appear in 18 files across ai/ and src/ — though it does push those two @returns lines well past the surrounding width.
• The PR body cites the projection sites as :361 and :527, which are the ticket's pre-deletion line numbers; post-change they sit at :333 and :499. Harmless, and it matches how the ticket referred to them.

Rhetorical-Drift Audit (per guide §7.4):

• PR description: framing matches the diff, and its central claim is measured rather than asserted — the twelve-case Object.is equivalence run is exactly the right instrument for a "behaviour is unchanged" claim
• Anchor & Echo summaries: the added clause is accurate and additive; the surviving helper's contracts are unaltered
• [RETROSPECTIVE]: N/A — none claimed
• Linked anchors: the "avoided trap" reasoning about collapsing toward the shared module correctly reproduces #17087's stated direction and the reason for it

Findings: Pass. The "What I could not run" section is the strongest thing in the body: naming an unexecuted surface instead of letting a partial local run imply full coverage is the discipline this repo asks for, and it is uncommon in a first PR.

🧠 Graph Ingestion Notes

• [KB_GAP]: None. The change shows correct reading of both the ticket's direction argument and the distinction between the shared module's contract prose and the service-local copy.
• [TOOLING_GAP]: Contributor-onboarding friction worth capturing, not a defect: a base npm install cannot run any ai/** spec, because those need the Brain tier (npm run install-brain → better-sqlite3, which needs a native toolchain — unavailable on this contributor's machine). So a first-time contributor on a good first issue whose entire proof lives in ai/** specs is structurally unable to run it locally and must rely on CI. The fail-closed CI guard means that is safe, but the ticket's "Note for a first-time contributor" section could say so up front, so the contributor knows the local skip is expected rather than a setup failure they should keep fighting.
• [RETROSPECTIVE]: The reusable pattern is answering an author's declared uncertainty with mechanism rather than reassurance. The author flagged two specs as unexecuted and asked CI to confirm. The useful reviewer move was not "CI is green, you're fine" but locating why green is trustworthy here — the brainTestMatch collection rule plus the assertBrainTierForEnvironment fail-closed guard, which converts "the specs might have been skipped" from an open question into a state the harness refuses to reach. A declared uncertainty deserves a mechanism, because "CI is green" would have been true and would have taught nothing.

N/A Audits — 📑 🪜 📡 🔗

N/A across listed dimensions: no OpenAPI/tool-description surface, no skill/convention/primitive touched, no public or consumed contract modified (the helper's signature and semantics are untouched; only the caller set grows), and the close-target's ACs are fully covered by existing unit specs, so no evidence-ladder declaration is required.

🎯 Close-Target Audit

• Close-targets identified: Closes #17087. in the body, and (closes #17087) in the title
• #17087 is not epic-labeled — labels are bug, good first issue, ai, refactoring, agent-os; it is a sub of #17072, which is correctly not named as a close-target
• Single delivered leaf, correctly scoped — the target issue is exactly what shipped, with no epic or broad reference smuggled in

Findings: The target is right; only the keyword deviates from house convention. This repo's convention for ai/ work is a newline-isolated Resolves #N — Closes / Fixes and a trailing period are the non-canonical forms, because the PR body doubles as graph-ingestion substrate and the parser keys on that exact shape. Concretely, the first line would become:

Resolves #17087

(no trailing period, on its own line). Worth noting for calibration: CI's lint-pr-body passed as-is, so this is a reviewer-side convention rather than a mechanically enforced one — which is precisely why a first-time contributor could not have known it. Not blocking: it is a body edit requiring no re-push and no CI re-run, and a maintainer can fold it in at merge just as easily as you can. I am naming it so the convention is learnable, not as a gate.

🧪 Test-Evidence & Location Audit

• Execution evidence: exact-head required CI green at 3c6585772b4aa50919e96ae210e1b36e437c7b6f — gh pr checks 20/20 pass, exit 0. Per the Depth Floor analysis, this necessarily includes the unit-brain project carrying both TimestampGuard specs.
• Reviewer falsifier: my named concern was "did the brain specs actually execute, or were they silently skipped" — the author had flagged exactly this. Falsified by reading the collection rule and the CI admission guard rather than by rerunning, since the guard makes the skipped-and-green state unreachable.
• Test location: no new spec file; the existing SummaryService.TimestampGuard.spec.mjs is correctly retargeted in its unit-level arm only, with every behavioural assertion byte-identical — which is what AC-3 asks for.

Findings: Pass. AC-by-AC, verified independently of the PR body's checklist: AC-1 the static is gone and both projections call resolveRowTimestamp (:333, :499); AC-2 git grep -n "resolveSummaryTimestamp" -- ai/ test/ returns no hits at this head; AC-3 only the direct-helper arm changed, assertions untouched; AC-4 both the absent-vs-unparseable contract and the new Date(null) → epoch-0 parity remain documented on the surviving helper.

📋 Required Actions

No required actions — eligible for human merge.

(The Closes → Resolves #17087 body edit noted above is optional polish, and the branch sits 5 commits behind dev, so a maintainer may want it brought current before merge. Neither affects the code.)

📊 Evaluation Metrics

• [ARCH_ALIGNMENT]: 98 — Collapses toward the shared module, which is the direction the ticket argues for and the only one that avoids creating importers of a service singleton's static. No new surface, no new file, no signature change; the helper was already generic over "any Chroma metadata row", so nothing needed to be made reusable first. 2 deducted only for the {@link} lines now running past the surrounding width.
• [CONTENT_COMPLETENESS]: 94 — Both load-bearing contracts survive on the helper, and the deleted copy's unique SUMMARY_QUERY_ERROR detail was folded in rather than discarded. 6 deducted for the PR body's non-canonical close-target keyword and its pre-deletion line references.
• [EXECUTION_QUALITY]: 97 — Behaviour is provably identical: same function body, and an Object.is equivalence run across twelve cases including the epoch-0 and unparseable branches. Both call sites converted, no assertion weakened, no consumer left half-migrated. 3 deducted because the third consumer's uncounted-malformed asymmetry, while correctly out of scope, is now visible and unticketed.
• [PRODUCTIVITY]: 100 — All four ACs delivered, each independently verified here, including the grep-based one returning zero hits.
• [IMPACT]: 45 — Deliberately modest and correctly so: no behavioural change and no defect fixed today. The value is removing a future-drift vector — the next person to alter the null → epoch-0 parity now has exactly one place to change, which is precisely the failure this duplication reproduced.
• [COMPLEXITY]: 20 — Three files, one deletion, two call-site swaps, one spec arm retargeted; the reasoning load was entirely in verifying invariance rather than in the change itself.
• [EFFORT_PROFILE]: Maintenance — A well-specified consolidation with no semantics change, where the substantive work is proving nothing moved.

Two habits here are worth keeping as you contribute further: measuring an invariance claim instead of asserting it, and naming what you could not run rather than letting a partial local run imply full coverage. Both are exactly what this repo asks of its maintainers.

— Grace (Claude Opus 5, Claude Code) 🖖