126 lines
8.1 KiB
Markdown
126 lines
8.1 KiB
Markdown
# Round-3 Verifier Report — commit 8996f7e
|
||
|
||
**Verifier:** fresh-context verifier session, 2026-06-12
|
||
**Commit under review:** `8996f7e238a8db040369188f9b5fa029394a56b9` — "feat(ux): judge round-2 fixes — store-level dedup roots, clean previews, honest totals, friendly agendas" (= current HEAD on main)
|
||
|
||
## VERDICT: PASS
|
||
|
||
All gates green (FE 944/944, server-local 920/920, weaver 31/31, tsc 0+0), every change traces to a
|
||
named judge complaint or the habit-forming mission, no new dependencies, no flags or compat shims,
|
||
and all four special-attention items check out. Three minor (non-blocking) findings below.
|
||
|
||
---
|
||
|
||
## 1. Diff audit — traceability, deps, flags
|
||
|
||
`git show --stat 8996f7e`: 79 files, +787/−55. Code surface: 10 FE files in `apps/web/src`,
|
||
5 server files (`monthly-assessment.ts`, `routes/{home,memory,skills,workspace-context}.ts`),
|
||
1 weaver file (`consolidation.ts`), 3 test files. The remainder is `judging/round2/*` evidence
|
||
(judge verdicts, prior verifier report, screenshots/crops) — process artifacts of this mission.
|
||
|
||
- **Traceability:** every code change maps to a complaint named in the commit message
|
||
(dedup root causes, honest totals, clean previews, jargon sweep, plural fix, future-only next-up,
|
||
recency sorting, fresh-install card preservation). Nothing speculative; no abstractions added
|
||
beyond a 7-line local `cleanPreview` helper and a 4-entry display-name map.
|
||
- **No new dependencies:** no `package.json` touched anywhere in the commit. `renderChatMarkdown`,
|
||
`parseSkillFrontmatter`, `deleteByContentPrefix` are all pre-existing in-repo utilities.
|
||
- **No flags/shims:** no new `process.env` reads, no compat layers. `FRIENDLY_JOB_NAMES`
|
||
(workspace-context.ts:126-132) is a presentation mapping, not a shim — verified its 4 keys
|
||
exactly match the 4 seeded cron names in `packages/server/src/local/setup-crons.ts:23-26`,
|
||
and unknown (user-created) names pass through unchanged.
|
||
- **Boundary validation only:** no new internal validation layers introduced.
|
||
|
||
### Special-attention items
|
||
|
||
**(a) weaver `deleteByContentPrefix` — cross-session safety: OK.**
|
||
`packages/weaver/src/consolidation.ts:218` deletes by prefix
|
||
`` `Session (${sessionDate}): ${summary}` `` — date AND summary are both in the prefix, so a
|
||
different session on the same day is untouched. The implementation
|
||
(`packages/hive-mind-core/src/mind/frames.ts:343-354`, pre-existing W4.3 utility) escapes LIKE
|
||
metacharacters (`\ % _`) and routes through `delete(id)` so FTS/vec/KG indexes are cleaned.
|
||
Scope is the weaver's own per-mind `FrameStore` — cross-workspace deletion is structurally
|
||
impossible. The new regression test explicitly distills a second, different-summary session on
|
||
the same date and asserts it survives (consolidation.test.ts:212-226). ✔
|
||
|
||
**(b) memory.ts stats all-minds aggregation — error tolerance: OK.**
|
||
`routes/memory.ts:423-434`: when no workspace is given, it loops `server.workspaceManager.list()`
|
||
through `countMind(getWorkspaceMindDb(ws.id))`. `getWorkspaceMindDb` → `mindCache.getOrOpen()`
|
||
(`multi-mind-cache.ts:40-82`) **returns `null` on any open failure** (caught + logged internally,
|
||
incl. the path-traversal guard), and `countMind` early-returns on `!wsDb` — so an unavailable
|
||
mind is skipped cleanly, per-workspace. The outer `try/catch` additionally covers enumeration
|
||
failure, degrading to the personal-only total. The route cannot 500 from a bad workspace mind. ✔
|
||
(Minor: a mind that opens but throws mid-query would abort counting of the *remaining*
|
||
workspaces — partial total, still no crash. See finding M2.)
|
||
|
||
**(c) monthly-assessment zero-data skip — real months not skipped: OK.**
|
||
`monthly-assessment.ts:298-304`: skip fires only when `totalInteractions === 0 &&
|
||
skillsInstalled === 0`. Any month with real interactions (or with skills installed even at zero
|
||
interactions) still writes its frame. The new test asserts a zero-data month writes nothing
|
||
(`FrameStore.getRecent(10)` length 0), and the adjacent pre-existing tests (which write months
|
||
with `totalInteractions: 142` etc.) all still pass in the 920/920 run. ✔
|
||
|
||
**(d) skills.ts cleanPreview — review-#3 frontmatter-leak protection: NO REGRESSION.**
|
||
The structure is unchanged from the review-#3 fix: the on-disk read + `parseSkillFrontmatter`
|
||
still overrides the raw-content default, so for every skill `loadSkills` returns (it only
|
||
returns files actually present in `skillsDir`), the preview is derived from
|
||
`frontmatter.description` (authored display text) or `parsed.body` — never the stamped
|
||
frontmatter. The raw-content fallback fires only on a read race (file deleted between the two
|
||
reads), identical to the pre-commit code. Decisively: the review-#3 regression test
|
||
(`p5-skill-governance.test.ts:71-79` — preview must not contain `initiator:` or `---`, must
|
||
contain the body heading text) is untouched by this commit and **passed** in the server-local
|
||
run. `cleanPreview`'s `#`-stripping keeps `toContain('Real Heading')` true. ✔
|
||
|
||
## 2. Test runs (executed by this verifier, HEAD = 8996f7e)
|
||
|
||
| Suite | Command | Result |
|
||
|---|---|---|
|
||
| Frontend | `npx vitest run --root apps/web` | **944 passed (944)**, 91 files, 0 failed |
|
||
| Server local | `npx vitest run packages/server/tests/local --root .` | **920 passed (920)**, 76 files, 0 failed |
|
||
| Weaver | `npx vitest run packages/weaver/tests --root .` | **31 passed (31)**, 3 files, 0 failed |
|
||
|
||
Zero failures — nothing to adjudicate. (Counts match the commit message's claimed gates;
|
||
"weaver 18/18" in the message referred to consolidation.test.ts alone — full weaver dir is 31.)
|
||
|
||
## 3. Typechecks
|
||
|
||
- `npx tsc --noEmit --project packages/server/tsconfig.json` → **0 errors** (exit 0)
|
||
- `npx tsc -p apps/web/tsconfig.app.json --noEmit` → **0 errors** (exit 0)
|
||
|
||
## 4. New tests read — do they assert the new behavior?
|
||
|
||
1. **Weaver re-distill** (`packages/weaver/tests/consolidation.test.ts:212-226`): distills the
|
||
same date+summary twice (key points evolving), plus a *different* summary same date; asserts
|
||
exactly 2 distilled frames for the date, exactly 1 for the re-distilled summary, and that the
|
||
survivor contains the updated `point B`. Asserts both replace-on-update AND no cross-session
|
||
deletion. ✔
|
||
2. **Assessment zero-data** (`packages/server/tests/local/monthly-assessment.test.ts:146-155`):
|
||
saves an assessment with `totalInteractions: 0, skillsInstalled: 0` and asserts the
|
||
FrameStore stays empty — exactly the skip behavior. ✔
|
||
3. **Briefing-highlights status filter** (`apps/web/src/lib/briefing-highlights.test.ts:27-37`):
|
||
feeds deprecated, archived, `User asked:`-echo, and one `active` frame; asserts only the
|
||
living frame survives. The `make()` helper spreads overrides so `status` flows into
|
||
`BriefingFrameLike` (which gained the `status` field in this commit). ✔
|
||
|
||
Brag-line tests (`login-briefing-brag.test.ts`) were also updated and assert the dropped
|
||
"across N workspaces" clause everywhere except the zero-state.
|
||
|
||
## 5. Findings (all minor, non-blocking)
|
||
|
||
- **M1 — theoretical prefix-collision in weaver dedup:** if two distinct sessions on the *same
|
||
date* have summaries where one is a strict string-prefix of the other ("Discussed Q2" vs
|
||
"Discussed Q2 marketing strategy"), re-distilling the shorter one would delete the longer
|
||
one's frame. LLM-generated summaries make exact prefix collisions unlikely; the cheap
|
||
hardening would be including the `. `/end separator in the delete prefix. Not a spec
|
||
violation — noted for awareness.
|
||
- **M2 — partial-total on mid-loop throw in stats aggregation:** the all-minds `try/catch`
|
||
wraps the whole loop, so one corrupt-but-openable mind aborts counting the remaining
|
||
workspaces (silently smaller total). Unavailable (unopenable) minds are handled per-workspace
|
||
via `getOrOpen → null`. Acceptable tolerance; per-workspace try would be stricter.
|
||
- **M3 — untested new server paths:** the all-minds stats aggregation, the home.ts
|
||
`hasContent` card filter, and `FRIENDLY_JOB_NAMES` have no direct tests (the commit's +5
|
||
regression tests cover the four most behavior-critical changes; skills preview is covered
|
||
indirectly by the pre-existing review-#3 test). Within the spirit of "add tests for new
|
||
interactive logic" but not exhaustive.
|
||
|
||
None of these alter the verdict.
|