feat: UX-5 Home metrics + the UX-1..UX-4 review fixes (20 defects) by kevintseng · Pull Request #184 · PCIRCLE-AI/memesh

kevintseng · 2026-08-17T11:58:10Z

UX-5 — Home leads with numbers that are true, and the review that got us there

Two things in one branch, because the second is what made the first
possible: a seven-way review of everything UX-1..UX-4 shipped, and the Home
metrics row those findings taught us how to build.

The review

Seven independent passes over PRs #157, #176, #181 and #183 — one per PR, one
cross-cutting on the seams, and two adversarial (one of them on a different
model, standing in for a Codex pass that hit its usage limit). Every finding
had to quote the source line that motivated it; anything unquotable was
forced to low confidence and kept out of the report.

Twenty defects were found and fixed. The ones worth reading about:

memesh why could be made to lie about provenance. The commit-hash
join used the stored entity NAME as a SQL LIKE pattern, where % and _
are wildcards — and the name is writable through the ordinary public API.
One entity named commit-%%%… answered for every hash and, because
ORDER BY length(name) DESC prefers the longest name, won deterministically.
Two independent reviewers reproduced it end to end: why returned planted
text as a commit's provenance and its abstention flipped from
no_commit_entity to an asserted memory. Parameters were always bound
correctly; binding does not constrain LIKE semantics over data an attacker
can write. Now a substring comparison plus a hex guard.
The C5 detector was blind to half of what it was written to catch. Its
regex ended in \b, and a word boundary cannot match after ] or } — so
|| true was caught and || [] / || {} never were, for the detector's
whole existence. Fixing it surfaced 10 occurrences it had never reported,
one of them a real defect (Memories' ranked search turning an unreadable
payload into "no results"). C1 had the same disease in a different spelling:
it could not see toHaveLength(N) as a size pin, and correcting that
cleared 12 baseline entries that were false positives all along.
Rule 5 linked evidence across projects, two ways. --project alpha
scoped the evidence query and not the work query; and on the default run a
shared session: tag was enough on its own, which matters because one
Claude Code session routinely touches two repos. Measured on a seeded
graph: 3 edges written, 2 across the boundary.
Two timestamp columns, two formats, one string comparison.
last_accessed_at is ISO-8601 and created_at is SQLite's space format;
comparing them as text ranked a recalled memory above a fresher
never-recalled one within any single day. Fixed at every cross-column site.
Deliberately NOT fixed by migrating the column — 781 rows are ISO, 0 are
not, and its readers are built for ISO: lifecycle.ts compares it against
a .toISOString() threshold, and under the other format every memory would
read as stale and have its confidence decayed.
The loop metric claimed a precision it never had. computedFrom flipped
to recall_hits whenever any in-window hit existed, which HID the "this is
an approximation" caveat — while the numbers stayed the approximation. On
the graph this was measured against, all 21 qualifying hits came from the
literal-matching accounting R1 retired at 0% signal.
Export lost every title. The bundle omitted the field and import never
read it, so a backup taken after UX-1 restored a library of machine keys.
Both halves fixed, including the Zod schema that was silently stripping it
on MCP and HTTP while the CLI worked.

Plus: why crashing with a stack trace on --limit abc and lying with
"that line does not exist" on --line abc; a git-log failure reported as
"No commits touch this file"; the graph drawing machine dedup keys where
UX-1's chain forbids it; graph search unable to find a node by the headline
it displays; the badge's draw and hit-test computing different geometry;
Signal Mode refetching the whole graph; Memories' ranked search with no
generation ticket; a truncation notice silenced by a health request that
never landed; Project reporting "no project memories yet" from a failed
fetch.

The metrics row

Two rules, both learned from the above:

Not measured is not zero. Citation compliance is the live example — the
counters do not exist until the release carrying R1 lands, so today every
install reads "tracking starts after your next session", not "0%".
Measured-at-zero is a different tile with a different sentence.

A number arrives with its denominator. Measured on a real graph: 29
lessons active, 12 classified, 5 critical. "5 critical" alone rounds the 17
nobody classified up into evidence they are not critical.

This is also where the retired Lessons tab's counters landed. Of its four,
only severity:critical is a number a reader acts on; the other three are
dropped, with the reason recorded in CHANGELOG rather than left as an
unexplained absence — and the recall total specifically because it was built
on the accounting since proven to carry no signal.

Verification

node scripts/run-tests-isolated.mjs   exit=0   Test Files 152 passed (152) / Tests 2220 passed (2220); no "Errors" line
npm run verify:release                exit=0

Every fix that could be pinned was, and each pin was mutation-verified —
revert the fix, watch a specific test go red, restore, re-verify green.

Two fixes are deliberately unpinned and said so rather than faked: the
graph's ego banner and its badge hit-test both need canvas pointer geometry
off a seeded layout, and a brittle test that passes for the wrong reason is
worse than a stated gap.

One test took four attempts to make real. Each earlier version passed
against the defect for a different reason: the mutation script's pattern
never matched; the query never fired because Preact had not flushed the
input's state; the finally-clause guard masked the removal of its two
siblings; and the assertion ran before the stale write could land. That
sequence is now recorded as the order to suspect when a mutant survives.

Follow-ups (not in this PR)

memesh kg backfill must run once for the evidence badges to populate on
an existing graph; the empty state and the docs both say so.
The Home row shows four tiles. The remaining work-topology metrics wait on
measurements that do not exist yet — which is the point.

…t have `computedFrom` describes the numbers in `loopMetric`, and both queries that produce them read `last_accessed_at` — nothing there reads `recall_hits`. The dashboard uses this field to decide whether to SHOW the "this is an approximation" caveat, so a wrong value here does not mislabel a number: it hides the sentence that explains it. It flipped to 'recall_hits' whenever the column existed and any knowledge entity had a hit inside the 30-day window. The comment on that gate named the exact failure it was written to prevent — "the badge would lie: say precise mode while the rendered numbers still come from the approximation" — and it did not prevent it, because a hit in the window is not evidence that these queries read hits. Measured on a real graph today: 21 knowledge entities carried in-window `recall_hits`, every one written by the literal-content matching that R1 retired at 0% measured signal, and `recall_accounting_mode` — the stamp that exists to keep the two accounting eras apart, which nothing read — was absent. The caveat was hidden on that graph. Earning the other value takes a schema change, not a probe: `recall_hits` is a per-entity running total, so a per-DAY reuse series cannot be derived from it. The comment says where to set the field when that query lands. [Verified-By: npx vitest run tests/transports/analytics.test.ts exit=0 — 'Tests 24 passed (24)'; npx tsc -p tsconfig.check.json --noEmit exit=0. Break-test: restoring the historical flip turns the new test red (1 failed), restored green]

…or that was blind to one class The C5 'optimistic default' detector could not see `|| []` or `|| {}`. Its regex ended in `\b`, and a word boundary cannot match after `]` or `}` — so `|| true` was caught and the two collection forms never were, for the detector's whole existence. Fixing it surfaced 10 occurrences it had never once reported. Nine are guarded (view-live checks `!data.success` before every one; serializer's field is genuinely optional) and are baselined with that reason. The tenth was real: - MemoriesTab's ranked search did `data.entities || []`, so an unreadable recall payload rendered as a successful search that found nothing. The load() path 100 lines above refuses that exact masquerade by name. Three more, all in code merged earlier today: - kg-backfill Rule 5 parsed SQLite timestamps with `new Date(v)`. The repo already owns that parse — time-utils.ts says so, and warns that engines accepting the format read it as LOCAL time. A uniform offset would cancel in a same-column comparison, but it is not uniform: measured under TZ=America/New_York, '2026-03-08 02:30:00' parses to 07:30Z and '2026-03-08 03:00:00' to 07:00Z, so the fallback's ordering INVERTS across the spring-forward hour. The same column can also hold a real ISO value (demo.ts writes one), read as true UTC beside its local-read siblings — an 8-hour skew between two rows of one table. Now routed through parseSqliteUtcMs, whose null means 'untrusted' and never sorts or anchors. - Rule 5's project fallback ignored maxEdgesPerSource, and `added === 0` is exactly what a cap of 0 produces — so `--max-per-source 0` wrote one edge per evidence entity while every other rule wrote none. - `memesh why` did not validate its numeric flags. `--limit abc` put NaN in a SQL LIMIT and crashed with a raw ERR_SQLITE_ERROR stack trace carrying the absolute install path; `--line abc` was worse than a crash — NaN reached git blame, the failure was caught, and the user was told 'That line does not exist in the tracked file', an affirmatively false statement from the one command whose contract is that it abstains rather than guesses. [Verified-By: node scripts/run-tests-isolated.mjs exit=0 — 'Test Files 150 passed (150)' / 'Tests 2193 passed (2193)', no Errors line; npm run verify:release exit=0. Break-test: reverting the timestamp parse and the cap guard together turns exactly 2 of the new tests red, restored green. CLI flags re-checked against a throwaway HOME: --limit abc / --line abc / --limit 0 each exit 1 with a sentence and no stack trace; --limit 3 exits 0]

…m a provenance hijack **why.ts: a stored entity name was used as a SQL LIKE pattern.** The join read `? LIKE substr(name, 8) || '%'`, which makes the NAME the pattern — and `%` and `_` are wildcards there, on data any caller can write: `remember --name 'commit-%%%…' --type commit` passes schema validation (the name field strips only control characters), and so does an import bundle. One such entity answered for EVERY hash, and `ORDER BY length(name) DESC` made it win deterministically, because real abbreviations are 7-40 characters and a name may be 255. Two independent reviewers reproduced it end-to-end through the public API, on the CLI and on the pure-DB `/v1/why` route: `memesh why` returned planted text as a commit's provenance and the abstention flipped from `no_commit_entity` to an asserted memory — the one thing this module exists not to do. The parameters were always bound correctly; binding does not constrain LIKE semantics over attacker-writable data. Now a substring comparison plus a hex guard, so a stored `%` matches only a literal `%`. **kg-backfill Rule 5 linked evidence across projects, two ways.** `--project alpha` scoped the evidence query and not the work query, so a run scoped to alpha wrote edges into bravo's decisions (measured on a seeded graph: 3 edges, 2 of them across the boundary). And on the default unscoped run, a shared `session:` tag was enough on its own — one Claude Code session routinely touches two repos and both get that tag, so a commit in one became evidence for a decision in the other. In UX-4 that renders as bravo's badge counting alpha's work. The session path now requires project agreement when both sides carry project tags, and stays permissive when either does not. [Verified-By: node scripts/run-tests-isolated.mjs exit=0 — 'Test Files 150 passed (150)' / 'Tests 2196 passed (2196)', no Errors line; npm run verify:release exit=0. Break-tests, each restored and re-verified green: reverting the LIKE join turns the wildcard test red (1 failed); reverting the two project guards turns the cross-project tests red (2 failed). Direct probe: the fixed join returns [] for an unrelated hash against seeded 'commit-%%%…' and 'commit-_______' rows, and still returns the real 'commit-abc1234' for its own hash]

…comparison `last_accessed_at` is written as `new Date().toISOString()` (storage/conflicts.ts) and `created_at` is SQLite's own CURRENT_TIMESTAMP. Comparing them as strings works until the date prefixes match, and then it compares 'T' (0x54) against ' ' (0x20) — so among entities touched on the SAME DAY, every recalled one sorts above every never-recalled one whatever the real times were. In the work-layer graph that is the exact inversion the view exists to prevent: a decision made at 23:00 and never recalled ranked below one last read at 09:00. Fixed by normalising the ISO column with SQLite's `datetime()` at both graph queries and at analytics' four window filters. Scoped honestly: across different days the date prefix already decided correctly, so this was same-day ties, not a total inversion. Deliberately NOT migrating the column to one format. Measured on a real graph: 781 rows ISO, 0 rows in SQLite's format — the column is internally consistent, and its readers are built for ISO. `recencyScore` parses it with `new Date()`, which is correct for ISO and would silently shift by the UTC offset if the stored form changed; `lifecycle.ts` compares it against a `.toISOString()` threshold, and under the other format ' ' < 'T' would make every memory look stale and decay its confidence. The cross-column comparison was the defect, not the storage. Two more in the same file: the work-layer relation query filtered status on neither endpoint while the entity query filtered both, so the payload carried edges to archived nodes the client then hid — leaving the live node drawn as connected with no visible edge. And the evidence drill-down ordered by `created_at` alone, which is second-granular, so a truncated page could differ between identical requests; it now breaks ties on id. The C1 detector could not see `toHaveLength(N)` as a size pin — it recognised only the `length).toBe(N)` spelling. Same blind-spot class as the C5 `\b` fixed in the previous commit. Correcting it cleared 12 long-standing baseline entries that were false positives all along. [Verified-By: node scripts/run-tests-isolated.mjs exit=0 — 'Test Files 150 passed (150)' / 'Tests 2199 passed (2199)', no Errors line; verification-audit exit=0. Break-tests, each restored and re-verified green: reverting the datetime() normalisation and the endpoint status filter turns 2 new tests red; reverting LIMIT cap+1 to cap turns the truncation-boundary test red]

…nters decision Three agents fixed disjoint file scopes; every claim was re-verified against the files before landing, and each agent reported honestly that its own fixes were unpinned. The tests here are that debt paid. **Dashboard.** The graph drew `entity.name` on the canvas, in the focus banner and in the drill-down heading — the machine dedup key (`pre-compact-<sessionId>`) that UX-1's chain exists so a human never has to read. Graph search matched only that key, so typing the headline you can see in the tooltip found nothing. The badge's draw and hit-test each computed their own geometry from a different radius, so on a hovered node the visible badge and its clickable circle were in different places; the draw now publishes where it drew and the hit-test reads that, deleting the second copy rather than syncing it. Toggling Signal Mode refetched the whole graph and closed an open drill-down, because the loader depended on a callback memoised on that flag. Memories' ranked search had no generation ticket: clear the box mid-search and the stale answer flipped the view back to ranked mode with an empty input. Its truncation notice read `health?.entity_count ?? 0`, so a health request that never landed silenced the notice entirely — a 12,000-memory library showed "2,000 active" and said nothing about the rest. Its sort compared an ISO timestamp against SQLite's space format, which ranked a recalled memory above a fresher never-recalled one within any single day. Project swallowed a failed `/v1/projects` into an empty array and told an established user they had no project memories; `fetchProjects` now throws on an unreadable payload instead of manufacturing one, and readmitted archived chain targets are filtered by project like the active side. **why.** Two new typed abstentions. `history_unreadable` replaces the `abstention: null` that made a git-log failure — output past execFileSync's 1 MB buffer, the 5s timeout, an unborn branch — print "No commits touch this file." `no_commits_supplied` separates "you sent no hashes" from "there are none", which `(data.commits ?? [])` had merged into one silent success. The session-entity query was unbounded and ran once per commit; it now caps in SQL and reports `truncated`, and the CLI prints that rather than showing 200 rows as if they were the session. **Export lost every title.** The bundle omitted the field and import never read it, so a backup taken after UX-1 restored a library of machine keys. Both halves are fixed, including the Zod schema that was silently stripping it on the MCP and HTTP paths while the CLI worked. **Lessons counters (KT's call).** Of the four the retired tab carried, only `severity:critical` is a number a reader acts on; it moves to Home and the other three are dropped, with the reason in CHANGELOG rather than left as an unexplained absence. It ships with its denominators — measured on a real graph, 29 lessons are active, 12 carry any severity tag and 5 are critical, so "5" alone would overstate what was classified. Citation compliance joins it as a tri-state whose `null` is today's honest answer: the counters do not exist until the release carrying them lands, and rendering that as 0% would report perfect non-compliance from an instrument never switched on. Still unpinned, deliberately: the ego banner and the badge hit-test both need canvas pointer geometry off a seeded layout, and a brittle test that passes for the wrong reason is worse than a stated gap. [Verified-By: node scripts/run-tests-isolated.mjs exit=0 — 'Test Files 151 passed (151)' / 'Tests 2211 passed (2211)', no Errors line; npm run verify:release exit=0. Break-tests, each restored and re-verified green: search-by-headline reverted → 1 red; recall generation ticket removed (all three guards) → 1 red; export title omitted → 1 red; git-log abstention reverted → 1 red (agent-run). The recall-race test took four attempts to make real — the first three passed against the defect because the query never fired, the finally-guard masked the removal, and the assertion ran before the stale write landed]

Four tiles above the fold: health, critical lessons, citation compliance, reuse. Two rules decide the whole component, and both exist because the alternative is a confident lie on the first screen anyone reads. **Not measured is not zero.** A tile whose instrument has never run says so, in words. It cannot render 0, because 0 is a measurement — "we looked and found none" — and printing it from an absence claims an observation nobody made. Citation compliance is the live example: the counters do not exist until the release carrying R1 lands, so on every install today it reads "tracking starts after your next session", not "0%". Measured-at-zero is a different tile with a different sentence. **A number arrives with its denominator.** Measured on a real graph: 29 lessons active, 12 carrying any severity tag, 5 critical. "5 critical" alone rounds the 17 nobody classified up into evidence that they are not critical, so the tile says "5 · of 12 classified · 29 lessons total". When nothing has been classified the value is not-measured, not zero, and an empty library says it has no lessons rather than that none are critical. The reuse tile keeps the approximation caveat its numbers earn — they come from last_accessed_at, which is what `computedFrom` has said since this morning's fix. The contract suite caught a real defect while this landed: `buildTiles` read `data.criticalLessons.severityTagged` unguarded, so a server one release behind threw during render — a white screen, since this app ships no error boundary. `isMetricsRenderable` now rejects a payload missing the groups the row reads and the row reports skew, rather than defaulting the groups into fabricated zeroes. `citationCompliance: null` passes that guard on purpose: null IS its answer. C1 also caught this file's own test asserting "zero tiles on failure" with no positive size pin — a component that rendered nothing would have satisfied it. The four-tile happy path is now asserted alongside. [Verified-By: node scripts/run-tests-isolated.mjs exit=0 — 'Test Files 152 passed (152)' / 'Tests 2220 passed (2220)', no Errors line; npm run verify:release exit=0. Break-test: folding both not-measured states into '0' / '0%' turns 3 of the 9 new tests red, restored green]

kevintseng added 6 commits August 17, 2026 11:15

kevintseng merged commit 25c82f2 into main Aug 17, 2026
13 checks passed

kevintseng deleted the feat/ux5-home-metrics branch August 17, 2026 12:16

Navigation Menu

Uh oh!

feat: UX-5 Home metrics + the UX-1..UX-4 review fixes (20 defects) - #184

feat: UX-5 Home metrics + the UX-1..UX-4 review fixes (20 defects)#184
kevintseng merged 6 commits into
mainfrom
feat/ux5-home-metrics

kevintseng commented Aug 17, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Uh oh!

Conversation

kevintseng commented Aug 17, 2026

UX-5 — Home leads with numbers that are true, and the review that got us there

The review

The metrics row

Verification

Follow-ups (not in this PR)

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant