Show the user their own rows at the end of hyp init

[bgmcmullen]

Setup ended on a suggested query that could not run (select count(*) from logs, against a dataset the install does not create) and on no evidence that capture worked at all. Both are now one thing: a rendered block of the user's own traffic, printed by the wizard and re-runnable as hyp query overview.

Closes #389
Closes #390

The block

Four sections - providers and models, sessions and tokens per day, repos, tools - counted in input/cached/output tokens rather than rows. A "part" is an internal grain (LLP 0026) that nobody outside the codebase should have to learn, and "3,412 parts" answers a question no one asked.

Bars are two-tone over input + output with cache excluded: cache is 98.9% of tokens, so including it makes every bar the same bar. Each table captions what its own bar charts, since they differ by section.

What HypAware has recorded
2026-07-03 to 2026-07-27 - showing 21 of 25 active days (47,341 of 70,471 rows); widen with --days 25

One planner, two overrun behaviors

Both callers share one plan: a timed probe measures this machine, then picks the widest window that fits a 5s / 150k-row budget. The chosen window is always stated, so a short one reads as a stated scope rather than a wrong answer. Measured against simulated slowness, the planner picks 31 / 27 / 6 / 2 days at 1x / 3x / 10x / 50x.

They diverge only when the plan turns out wrong:

• The wizard holds a deadline (budget + 3s) and renders whatever finished, naming the unfinished sections as unfinished rather than omitting them. A stall at the end of a successful install reads as a broken install.
• hyp query overview has no deadline. There the user asked and is watching; no answer is worse than a slow one, and they hold --days either way.

Nothing in the wizard's half can fail setup: the whole step - queries, render, and write - sits inside one try, and the caller discards the result. An EPIPE from hyp init | head used to escape and exit non-zero from an install that had already fully succeeded.

Disclosure

Withheld rows (LLP 0105) are reported on both surfaces, to stderr. A block that quietly drops rows and reads as a complete picture is the failure that LLP exists to prevent, and it lands harder here than on a hand-written query: the user typed no filter and has no reason to suspect one. The five statements dedup to one sentence, worded by the query verb's own renderLocalOnlyNotice so the two surfaces cannot drift apart.

The freshness line is dropped in the wizard only - mid-install it names a sub-two-minute lag the user did not cause and cannot act on, on a block whose backfilled rows were force-flushed on the way in.

Bug fixed along the way

An unguarded cast(cache_read) + cast(cache_write) goes NULL for every OpenAI row (they omit cache_write_tokens), silently zeroing their cache reads - 25.5M of them on my machine. Per-term coalesce, pinned by a test, and corrected in LLP 0035's own canonical SQL and in both report skills, which carried the same shape.

Not in scope

#398 (Codex rows carry no repo_root) stays open. The repos section counts those in a footnote line rather than dropping them.

Verification

2696 tests pass, npm run typecheck clean, and all 13 smoke flows from the release checklist green.

🤖 Generated with Claude Code

[philcunliffe]

Review round 1 - 0e29586

Verdict: findings, no blockers. Reviewed under neutral:review (review-only, LLP 0032), so nothing here was pushed to the branch. Three independent passes: query/SQL correctness, wizard integration and failure containment, repo conventions and LLP discipline.

This is a strong PR. The headline bug fix is real and properly pinned, the disclosure plumbing does what the description says, and the tests exercise real failure paths rather than mirroring the implementation. The findings below cluster on one theme worth taking seriously given what this feature is for: a block whose purpose is an honest picture of the user's own data has several places where it quietly under-reports.

Verified sound

The cache-read fix is real, complete, and pinned. Reverting SUM_CACHED (overview.js:55-57) to the unguarded cast(read) + cast(write) form makes test/core/query-overview.test.js:619 fail with cached_tokens: 0 against an expected 5 on the OpenAI row, and that test runs through the real executeQuerySql seam rather than a stub. Grepped the tree: SUM_CACHED is the only cross-term addition, and LLP 0035's canonical SQL plus all four skill copies were corrected to match.

Containment holds. No await sits outside the try; the only statement outside it (runner.hasDataset(), first_look.js:125) is itself wrapped in the production runner. withSpan rethrows and index.js:176 does not catch, so containment rests entirely on that inner try, which does hold for synchronous throws, render errors, and query rejections. We tried to construct a leak and could not.

No deadline leak or hang. The setTimeout at first_look.js:92 is cleared in finally on both paths. The abandoned collectOverview keeps running but cannot hang hyp init (bin/hypaware.js:59 exits unconditionally) and cannot write to stdout. Promise.race attaches a reject handler to the abandoned promise, so a late failure is handled rather than unhandled.

Disclosure (LLP 0105) is correctly wired, not just unit-tested in isolation: say() fires on the same run() that returns the rows, deduped per runner, worded by the shared renderLocalOnlyNotice, and the sink is actually connected at index.js:175.

Conventions and LLP discipline are good. All 18 added @refs resolve to live anchors (including 0035#null-union, minted by this same PR). Living-docs rule satisfied in a single commit. Every added @import is correctly repo-root-anchored at the right depth. No @typedef, no added semicolons. Token math, date boundaries, divide-by-zero, overflow, and --days injection were probed and came back clean.

Scope honesty on #398 is real: the footnote exists, is accurate, is tested (query-overview.test.js:701), and LLP 0135:509-518 documents the measured asymmetry rather than hiding it.

Findings

1. NON-BLOCKING, and the one I would fix first. The repos fold count is wrong, and the "no repo recorded" line can vanish entirely. overview.js:106 selects with limit 20; the renderer shows MAX_REPO_ROWS = 8 (:218) and computes hidden = named.length - shown.length (:737) from what SQL returned. So "+ N more repos" saturates at 12: with 30 repos in the window the user is told 12 are hidden when 22 are. The repo-less group (repo_root null) competes for the same 20 slots ordered by token volume, so on a machine with more than 20 repos it can be dropped by the engine and the "+ N sessions with no repo recorded" line disappears. That directly contradicts the comment at :100-103 ("folded into a count line by the renderer ... so the total stays reconcilable"). Reproduced with a 20-row repo fixture. Fix: drop the SQL limit and fold in the renderer, or add a tail aggregate so counts are computed over all groups.

2. NON-BLOCKING. The daily table truncates at 14 rows with no disclosure. overview.js:95 ends limit 14, but unlike the models and repos tables there is no fold line. Follow the block's own advice (widen with --days 30, :591) and the header states 30 active days while exactly 14 day rows print. The skill docs this PR adds tell agents to read that header and trust it, so a reader summing the daily column silently gets 14/30 of the period. Fix: add a fold line, or set the limit from the chosen window length.

3. NON-BLOCKING. "Nothing recorded yet" is false when every row was withheld. overview.js:550-553: if the LLP 0105 filter withholds everything, the probe returns nothing and stdout prints "Nothing recorded yet. Start a session in a client you attached..." while stderr simultaneously reports the withheld rows. Verified end to end. The test at query-overview.test.js:726 uses an either-or match (/Nothing recorded yet|What HypAware has recorded/) so it does not pin the wording. Fix: a distinct "rows were withheld" empty state.

4. NON-BLOCKING. The EPIPE claim in the description is not what the code does, and the test backing it proves the wrong thing. first_look.js:135-140 says the try contains "EPIPE when stdout is a closed pipe", and first-look.test.js:169-191 builds a stdout whose write() throws synchronously. Real Node stdout on a pipe is a net.Socket: EPIPE arrives asynchronously as an 'error' event, which bypasses every try/catch in the process. Running the real runWizardFirstLook against a real closed pipe raises an uncaught Error: write EPIPE at first_look.js:173:16 and exits 1. Two things keep this off blocker status: the cited trigger is unreachable (init.js:96 gates the wizard on isTty, so hyp init | head never enters it), and the behavior is pre-existing and CLI-wide rather than a regression. But hyp query overview | head -20 is a new surface (query.js:188, no try around the write) and does dump a stack trace and exit 1. Fix: either handle 'error' on stdout/stderr in bin/hypaware.js and keep the claim, or correct the comment and the test to describe what is actually contained. A test asserting EPIPE containment needs a real pipe.

5. NON-BLOCKING. --days=7 is silently ignored. query.js:97-105 hand-parses argv via indexOf('--days') instead of the repo's argvToParams (verb_codec.js:154-157), which handles the --flag=value form and rejects unknown flags. Verified: hyp query overview --days=7 --bogus runs the auto-planned window and exits as if nothing was passed. The user pinned a window, got another, and only a close read of the header would reveal it. The rest of the --days validation is solid (0, -1, abc, 3.5, missing all exit 2 with a clear message).

6. NON-BLOCKING. Output is 101 columns wide with no terminal adaptation. overview.js:221 (BAR_WIDTH = 18), :548, :850. Nothing reads stdout.columns, though tui/runtime.js:173 provides a helper. On an 80-column terminal (common over SSH and in split panes) every token table wraps mid-row and the alignment this module exists to produce is destroyed, at the closing moment of hyp init.

7. NON-BLOCKING. NO_COLOR is ignored. wizard/index.js:179 and query.js:188 gate colour on isTty(stdout) only, though the repo honours NO_COLOR at tui/runtime.js:37. Non-TTY degradation is correct; NO_COLOR=1 on a TTY still emits ANSI.

8. NON-BLOCKING. Style violations in new content. test/core/query-overview.test.js:47 uses an inline @type {import('...')}, which CLAUDE.md forbids outright (declare @import at the top, reference the bare name). And six em dashes are added across hypaware-core/plugins-workspace/{claude,codex}/skills/hypaware-query/SKILL.md:31,33,82. In the contributor's favour: the other ~2,900 changed lines are scrupulously em-dash-free, and those two files already carry ~21 pre-existing em dashes each, so the additions match local surrounding style rather than disregarding the rule.

9. NON-BLOCKING, cosmetic. Post-deadline disclosure can print after the privacy narration. collectOverview (overview.js:487-494) has no cancellation signal, so a section resolving after the deadline can still write a local-only: withheld N row(s) line to stderr after runWizardFirstLook returned. That contradicts the documented "privacy narration stays the last words" invariant (wizard/index.js:174, first_look.js:144, LLP 0135#first-look). It errs toward over-disclosure, so LLP 0105's substantive guarantee is not weakened, and it needs both a deadline expiry and withheld rows first appearing post-deadline. Fix: thread an AbortSignal, or gate the sink once the step returns.

10. NON-BLOCKING, hardening only. since is interpolated into five statements (overview.js:85). No exploit is reachable today: the sole writer of date is message_projector.js:492 (utcDate, always toISOString().slice(0,10)), and --days never reaches SQL. But the value round-trips out of the cache and the executor takes no bind parameters, so the only guard is a projector invariant in another package. A /^\d{4}-\d{2}-\d{2}$/ assert in buildOverviewSql would close it.

Note on the test suite, independent of this PR

npm test on this branch is 2688 pass / 7 fail, all in test/core/leave-command.test.js. Those failures reproduce on a clean origin/master tree (8 fail there), so they are pre-existing and untouched by this PR, which changes no leave or central code. CI is green and is the authority. Flagging it because the local suite is not currently green on master either, which is worth knowing on its own.

Verification run

Three isolated detached worktrees at 0e29586. npm run typecheck clean in all three. node --test on the changed test files: 102 pass, 0 fail. Mutation check on the coalesce fix as described above, reverted afterward. Ad-hoc probes covered chooseOverviewWindow with days: 0 and -3 (throws TypeError on an exported function whose JSDoc types days?: number with no lower bound, though it is unreachable from the CLI), a 30-day window against the 14-row daily limit, a 20-row repo fixture against the fold count, formatCount at 2^53, renderOverview with all-zero and null token columns, runQueryOverview --json with 9e15-token rows, a real .hypignore local-only directory, hyp query overview --help and seven --days inputs, a real closed pipe against runWizardFirstLook, and measured render widths. All 18 added @ref anchors resolved manually (.claude/skills/ref-check is a prompt-based skill with no executable script).

---

Requesting changes on the aggregate of findings 1 through 5, which are the ones that make the block report something other than what is true. This verdict is advisory: neutral does not merge or ready contributor PRs, and the maintainer decides. If you would rather land this as-is, these are all follow-up sized, and I am happy to see them ride a tracking issue instead. Reply on this thread either way and neutral picks it up on its next tick.

[philcunliffe]

Reviewed at 0e29586 under neutral:review (review-only, so nothing was pushed). Full findings are in the review record comment above.

No blockers. The cache-read bug fix is real, complete across the tree, and pinned by a test that runs through the real query seam. Containment, the deadline machinery, disclosure wiring, and LLP discipline all check out.

Requesting changes on five findings that make the block report something other than what is true, which matters more than usual on a feature whose whole purpose is an honest picture of the user's own data:

1. overview.js:106 - repos fold count saturates at 12 (SQL limit 20 vs MAX_REPO_ROWS = 8), and the "no repo recorded" line can be dropped entirely on a machine with more than 20 repos. Contradicts the comment at :100-103.
2. overview.js:95 - the daily table truncates at 14 rows with no fold line, while the header states the full window.
3. overview.js:550 - "Nothing recorded yet" prints when every row was withheld, while stderr says rows were withheld.
4. first_look.js:135-140 + first-look.test.js:169-191 - the EPIPE claim does not match Node's behavior (EPIPE on a pipe is an async 'error' event, not a synchronous throw), and the test's stub throws synchronously. hyp query overview | head stack-traces and exits 1.
5. query.js:97-105 - --days=7 is silently ignored, and unknown flags are not rejected, because argv is hand-parsed instead of going through argvToParams.

This verdict is advisory. neutral does not ready or merge contributor PRs, so the call is yours: if you would rather land it as-is, all of these are follow-up sized and can ride a tracking issue. Reply here either way and neutral will pick it up next tick.

[bgmcmullen]

Thanks - this was a genuinely useful review. The clustering observation was the right read: findings 1, 2, and 3 are the same mistake in three places, a block whose purpose is an honest picture quietly reporting something narrower. All five are fixed in 256dfc3, plus 7, 8, 9, and 10. Two are deferred with issues rather than decided unilaterally, and one claim I could not reproduce.

Fixed

1 - repos fold count. Confirmed exactly as described: 30 repos reported "+ 12 more". Dropped the limit 20. The grouping was computed in full either way; the LIMIT only decided how much of it the renderer got to see. The repo-less eviction you spotted is the sharper half - it takes the "+ N sessions with no repo recorded" line with it, which is the disclosure that makes the #398 scope note honest. Test pins both: 30 repos plus a repo-less group renders "+ 22 more repos" and keeps the repo-less line.

2 - daily truncation. Same fix, and I agree it is the worse of the two because the header states the window right above the table. limit 14 gone from SQL; the renderer shows MAX_DAY_ROWS and states what it folded. Live: --days=25 now prints "+ 11 earlier days in this window".

3 - false empty state. The runner records whether the LLP 0105 filter withheld anything (sawWithholding()), and the renderer picks the sentence that is true: "Every recorded session in this window is marked local-only and not visible from here." You were right that the either-or assertion pinned nothing - it now asserts the withheld wording and asserts "Nothing recorded yet" is absent.

5 - --days=7. Confirmed, including that --bogus was accepted silently. Routed through parseCommandArgv with a schema, which this file already uses for query maintain. I took minimum: 1 on the schema rather than keeping my hand-rolled range check, so the wording matches every other command ("--days expects a positive integer (got 0)"); tests updated to that.

7 - NO_COLOR. Added useColor(stream, env) beside isTty in stdio.js and used it at both call sites.

8 - style. Both fixed. On the em dashes: your read is the charitable one but it is still the rule, and matching local style is how a file stays non-compliant forever.

9 - late disclosure. Went with gating over threading a signal: the sink now closes when the step returns. An AbortSignal through collectOverview is the better fix and is worth doing when the deadline is threaded into executeQuerySql's existing signal (already noted as the next step in LLP 0135 #overrun).

10 - since interpolation. Added the /^\d{4}-\d{2}-\d{2}$/ assert. Your framing convinced me: "an invariant maintained in another package" is the wrong thing for a string concatenated into SQL to rest on, independent of whether it is reachable today.

Corrected rather than fixed

4 - EPIPE. You are right that the comment overclaims, and I have corrected it and the test to state only what they pin (synchronous write failures). But I could not reproduce the crash, on either surface:

• hyp query overview | head -1 (and -2, -5): clean exit, no stack. The block is 3,954 bytes, well inside a 64 KB pipe buffer, so the write completes before the reader's exit can matter.
• hyp query sql "select * from ... limit 3000" | head -1: also clean - the row-data budget caps the payload before it can exceed the buffer.
• A synthetic dead pipe from a harness: no uncaught error either.

So the structural point stands and the fix belongs CLI-wide, but hyp query overview | head -20 does not currently dump a stack. Filed as #409 with those measurements; the comment now names the issue and states why it is unreachable here on two counts (the wizard runs only under isTty, and the payload fits the buffer). Not fixed in this PR because installing an 'error' handler in bin/hypaware.js changes every command's behaviour and wants its own tests against a real pipe.

Deferred with an issue

6 - terminal width. Confirmed and worse than reported: 132 columns here, not 101. The bar is not the problem - shrinking BAR_WIDTH from 18 recovers at most 14 columns, so 132 to 118, still well over 80. The width is the counts themselves; 1,169,377,480 is 13 characters and there are three such columns. Getting under 80 means compact units (1.2B / 575M), which takes the widest line to about 70 but changes what the block is. That is a design call, so it is #410 with the options laid out rather than something I decide inside a review fix.

Verification

2707 tests pass (was 2696; 11 added), typecheck clean, all 13 smoke flows from the release checklist green. LLP 0135 gains a #folds section documenting why those two statements carry no LIMIT, since that is exactly the kind of thing someone re-adds as an optimization.

On the pre-existing leave-command failures: agreed, untouched by this PR and worth knowing separately.

[bgmcmullen]

Correcting a measurement in my previous comment: I reported the block as 132 columns wide. That was wrong. awk counts bytes, and the bar glyphs (▒ █) are 3 bytes each, so an 18-cell bar was counted as 54. The character width is 96, with 9 of 64 lines over 80 - closer to the 101 originally reported in the review than to my figure.

This changes the conclusion on finding 6. I argued compact units (1.2B) were unavoidable because shrinking the bar could not close a 52-column gap. The gap is 16, and the bar alone nearly closes it: 18 cells to a 4-cell floor saves 14, and the existing name ellipsis covers the rest. Exact digits stay, which is the right outcome - the counts are the block's primary content.

#410 has been rewritten with the correct measurement, the per-column breakdown, and a concrete fix (thread stdout.columns into renderOverview, give renderTable a width budget, shrink the bar first). It is now a contained change rather than a design decision, so it no longer needs a call from you before someone picks it up.

[philcunliffe]

Review round 2 - 256dfc3

Verdict: findings, one blocker. Nine of the ten round 1 findings verified as genuinely fixed, not moved or masked. The blocker is an interaction between two of those fixes, and it was introduced in this round.

First, the correction I owe you.

Round 1's EPIPE finding was wrong

"hyp query overview | head -20 dumps a stack trace and exits 1" does not reproduce, at either head. Your explanation is correct, and round 1's claim came from a test stub that threw synchronously rather than from a real pipe. Measured in isolation (Node 22.23.1, Linux, 64 KB pipe buffer):

stdout writes	bytes	piped to head -1
1	4,000	clean, exit 0
1	70,000	EPIPE stack, exit 1
2	4,000 each	EPIPE stack, exit 1

The crash needs a single write over the buffer, or a second write after the reader is gone. runQueryOverview writes the block in exactly one stdout.write() and the renderer folds every section, so the text block stays ~1.7 to 3.6 KB regardless of cache size. It can hit neither condition. Confirmed through real dispatch at both heads, including 400 repos by 60 days at round 1's head: clean, exit 0. Your correction to the comment and the test is right, and #409 is the right home for the structural gap.

Findings

1. BLOCKER. The new withheld-rows empty state tells the user to run a flag that hyp query overview rejects. overview.js:592-596 ends with Re-run inside one of those directories, or with --include-local-only. That flag is a query sql verb flag (verb.js:36); QUERY_OVERVIEW_SCHEMA (commands/query.js:84-92) declares only json, sql, and days, and overviewRunnerFromCtx never passes includeLocalOnly to executeQuerySql. Round 2's parseCommandArgv migration (fix 5) turned what would have been a silent ignore into a hard refusal, so the two fixes collide:

$ node bin/hypaware.js query overview --include-local-only
hyp query overview: unknown flag --include-local-only
usage: hyp query overview [--json] [--sql] [--days <n>]
exit=2

Reproduced end to end through real dispatch with one fixture row under a .hypignore local-only directory and the caller outside it: stderr says "rerun with --include-local-only", the block itself repeats the advice, and following it exits 2.

Failure scenario: a user whose recorded sessions all live under a local-only directory runs hyp query overview, or simply finishes hyp init, since the same block closes the wizard. They are told twice to re-run with a flag that does not exist on this surface. The fix whose stated goal was "the renderer picks the sentence that is true" ships a sentence whose second half is not, and query-overview.test.js:734 (assert.match(stdout.text(), /--include-local-only/)) now pins the wrong advice, so it will not self-correct.

Fix, either: drop the clause from overview.js:594 and suppress or reword the renderLocalOnlyNotice override on this surface, or add 'include-local-only' to QUERY_OVERVIEW_SCHEMA and thread it through to executeQuerySql. Update the test either way. Minor, same line: it says "in this window" when a fully withheld probe means no window line was printed at all.

2. NON-BLOCKING, but it changes what #409 says. Dropping the SQL LIMITs made --json unbounded, and that does cross the pipe buffer. overview.js:106-108,125-127 plus commands/query.js:203. The text renderer folds, so fix 1 and 2 are safe there. But --json dumps repoRows and dailyRows whole, and those are exactly the two statements whose LIMIT was removed. Measured through real dispatch:

input	round 1 head 0e29586	round 2 head 256dfc3
--json, 400 repos x 60 days	8,609 B, | head -1 clean	89,868 B, uncaught EPIPE, exit 1
--json, 30 repos x 730 days	(capped by the limits)	117,177 B, uncaught EPIPE, exit 1
text, any size	~3.6 KB, clean	~3.6 KB, clean

hyp query sql cannot do this: it is capped by DEFAULT_QUERY_MAX_BYTES = 32_768. overview --json has no such bound and was, until this round, incidentally bounded by the SQL limits. So on a cache spanning about two years, hyp query overview --json | head -20, or | less with an early quit, prints a stack trace and exits 1.

This does not undo fix 1 or 2. The #folds reasoning in LLP 0135 is sound and the scan cost is unchanged (at an absurd 20,000 distinct repos the whole run is 18.8s vs 15.2s, dominated by the scan; at realistic cardinality there is no measurable difference). The point is narrower: #409's "no live reproduction" note is now false, and the measurements above belong on that issue. A byte cap on the --json payload would close it in-PR; otherwise #409's stdout 'error' handler should land before users with long histories hit it.

3. NON-BLOCKING, latent. overview.js:309 declares let withheld = false (the flag sawWithholding() returns) and :338 declares const withheld = renderLocalOnlyNotice(...), a string, inside run. No defect today, since say() is a separate closure and nothing reads withheld inside run above line 338. But any future line inside run that touches it above 338 gets a TDZ ReferenceError, in the function that decides whether an LLP 0105 disclosure happens. Worth renaming one.

Round 1 fix verification

#	Verdict
1 repos fold	PASS. limit 20 gone (:127); hidden computed from the full named set, loose from all repo-less rows. Pinned at query-overview.test.js:797: 30 repos plus a repo-less group gives + 22 more repos and + 41 sessions with no repo recorded, and asserts + 12 more is absent.
2 daily fold	PASS. limit 14 gone (:108); MAX_DAY_ROWS plus + N earlier day(s) in this window (:735-738). Pinned both ways (30 rows folds, 3 rows prints no line). Bonus catch: Math.max(...) now spreads shown rather than the unbounded array.
3 false empty state	PASS on mechanism, defective on wording. sawWithholding() is a per-runner closure created fresh in overviewRunnerFromCtx, so no cross-run leak is possible, and both callers build a runner per invocation. But see finding 1.
4 EPIPE	Round 1 was wrong, see above. Comment and test correctly narrowed; #409 open and accurate.
5 --days	PASS. Verified against the real CLI: --days 7 and --days=7 both work, 0/-1/abc/3.5 exit 2 with "expects a positive integer (got 0)", --bogus exits 2, a bare positional exits 2, --help exits 0. No previously-valid invocation broke; newly refused flags (--refresh, --format) were silently ignored before, which is an improvement.
6 terminal width	Deferred legitimately. #410 open, and it corrects itself: 96 columns, explicitly retracting the 132 figure because awk counts bytes and the bar glyphs are 3 bytes each.
7 NO_COLOR	PASS. useColor (stdio.js:21-24) at both sites, isTty semantics unchanged, every other isTty call site untouched, and empty-string NO_COLOR correctly does not disable colour (matches no-color.org). Pinned for the query surface; the wizard call site has no test.
8 style	PASS. @import { OverviewWindow } at the top of the test file; zero em dashes on any added line in the PR diff.
9 late disclosure	PASS, and no suppression regression (the thing I most wanted to check). notices.close() (wizard/index.js:186) runs strictly after runWizardFirstLook resolves, and every notice for a rendered section is emitted synchronously inside runner.run() during the awaited collectOverview, so no in-window disclosure can be dropped. A section still in flight contributes no rows and is named as unfinished.
10 since assert	PASS. /^\d{4}-\d{2}-\d{2}$/ at :91-93, with '' allowed for the no-window render. Tested with an injection payload and a bare word; both callers catch, so a malformed cached date degrades to a skipped step rather than a stray query.

Verification run

npm test at this head: 2707 tests, 2699 pass, 7 fail, 1 skipped. npm run typecheck exit 0. Targeted node --test across the four changed test files: 113 pass, 0 fail.

Baselines from clean git archive trees: merge-base 78447ae 2637 tests / 7 fail; round 1 head 0e29586 2696 / 7 fail; this head 2707 / 7 fail. The failure count did not grow at any point, all in test/core/leave-command.test.js, still the only failing file. Your "2696 to 2707, 11 added" is accurate.

One calibration note on those pre-existing failures: the "8 failures" figure quoted earlier is master's. ae63a02 has 11 tests in that file with 8 failing; this branch forks from 78447ae and has 10 with 7 failing. Master added one more failing leave test the branch has not picked up. Unrelated to this PR either way.

Deferrals ground-truthed with gh issue view: #409 open and matching, #410 open and matching.

---

Requesting changes on finding 1 alone. Findings 2 and 3 are yours to weigh, and finding 2 may be better handled as an update to #409 than as work in this PR.

This is round 2, which is neutral's cap, so it will not keep re-reviewing. The verdict stays advisory: neutral does not ready or merge contributor PRs, so landing this is your call and the maintainer's. For what it is worth, the response to round 1 was unusually thorough, and finding 1 is the kind of defect that only appears when two independently correct fixes meet.

[philcunliffe]

Round 2 reviewed at 256dfc3. Full findings in the review record above.

One blocker, and it is an interaction between two of your round 1 fixes rather than a mistake in either: the new withheld-rows empty state (overview.js:592-596) advises --include-local-only, but QUERY_OVERVIEW_SCHEMA (commands/query.js:84-92) does not declare it, and the parseCommandArgv migration turned that from a silent ignore into exit 2, unknown flag. A user whose sessions are all local-only is told twice to run a flag that refuses. query-overview.test.js:734 currently pins the wrong advice.

Nine of ten round 1 findings verified as genuinely fixed, including the one I most wanted to check: gating the notice sink does not suppress any in-window disclosure.

Round 1's EPIPE finding was wrong and I have corrected it in the record. It does not reproduce at either head; your buffer explanation is right, and the false positive came from a test stub that threw synchronously. One thing to add to #409 though: dropping the SQL limits made --json unbounded, and that payload does cross the 64 KB buffer (89,868 B at 400 repos x 60 days, uncaught EPIPE, exit 1). So #409's "no live reproduction" note is now out of date.

This is round 2, neutral's cap, so it will not keep re-reviewing. The verdict is advisory as always: neutral does not ready or merge contributor PRs, so whether this lands is your call.

[bgmcmullen]

Round 2 addressed in 64adc0d. Both findings were right, and the second one was right about me being wrong, which I appreciate the follow-up on.

The blocker

Fixed by supporting --include-local-only, not by softening the advice - the disclosure is correct that this is how you see those rows, so the flag is what should change. It is worse than the empty state alone, too: the withheld-rows notice on stderr names the same flag, so the block was recommending it on every withheld run, not only the all-withheld one.

Declared on QUERY_OVERVIEW_SCHEMA with the same consent wording as the query sql verb (so the informed-consent language cannot drift between surfaces), threaded through overviewRunnerFromCtx to executeQuerySql. The wizard does not pass it and cannot - nothing in a setup step should quietly widen what a captured transcript can carry, and whoever reads the notice can run the command themselves.

Tested both directions: without the flag the rows are withheld, disclosed, and the empty state names it; with it, the rows appear and stderr is silent. Plus a test that the usage line lists every flag the codec accepts, since the failure mode here was precisely a gap between what the output advises and what the parser takes.

The EPIPE correction

You are right and I was wrong, and the interesting part is that the finding became true because of round 1. My "no live reproduction" was measured on payloads under the ~64 KiB buffer, where the write completes before the reader's exit can matter. Dropping the SQL limits made --json unbounded - the counts behind the fold lines have to be exact - and that is what crosses it.

Measured here, both directions against a real pipe:

	exit	stderr
400 KB into a closed reader, no handler	1	Unhandled 'error' event + stack
same, with the handler	7 (the command's own)	empty

So I fixed it rather than leaving it in #409, which is now closed with that correction. bin/hypaware.js installs a stdout/stderr 'error' listener before anything writes (src/core/cli/stream_errors.js). EPIPE is swallowed - a reader closing the pipe is | head saying enough - and the run finishes its normal path, observability shutdown included, exiting with the code the command chose.

test/core/cli/stream-errors.test.js pins it against a real pipe, including a test that the unpatched path is fatal, so the protective test cannot quietly stop testing anything. That is the real-pipe test round 1 correctly said a synchronously-throwing stub could not stand in for.

The first-look comment no longer claims async EPIPE is unreachable. It says where the handling lives and why it can never be a command's to catch.

State

2714 tests pass (7 added), typecheck clean, 13/13 smokes green. LLP 0135 #disclosure records why the override has to exist wherever the disclosure names it.

Understood on round 2 being the cap - no further review needed from your side. #410 (terminal width) stays open as the only deferred item; it is cosmetic, does not bite above 96 columns, and its body now carries the corrected measurement and a worked fix.

[philcunliffe]

Review round 3 - 64adc0d

Verdict: findings, no blockers. The round 2 blocker is genuinely fixed, verified end to end against the real CLI and a real cache. The consent surface holds up under scrutiny. The EPIPE fix is real, correctly scoped, and changes no exit codes. Nothing in the round 3 delta regressed a default open.

The single finding is a carry-over nit that is not a live defect and should not hold the PR.

The blocker is fixed, and fixed the right way

Verified end to end with a real fixture (temp HYP_HOME, @hypaware/ai-gateway plus @hypaware/claude activated, a Claude transcript backfilled with cwd inside a .hypignore local-only directory), running the real bin/hypaware.js from a sibling directory:

• Without the flag: exit 0, stdout carries the withheld empty state, stderr carries local-only: withheld 2 row(s) ... (rerun with --include-local-only ...).
• With the flag: exit 0, the withheld rows appear, stderr is empty.
• From inside the local-only directory: rows appear, no disclosure, so the other remedy the empty state names is also true.
• Mixed case (some visible, some withheld, 60-day fixture): stderr carries the notice while stdout carries the block, and re-running with the flag surfaces the withheld session.

So the advice is now true on every path that gives it, including the stderr notice, which was the broader half of the bug you spotted. The usage line reads usage: hyp query overview [--json] [--sql] [--days <n>] [--include-local-only], and an adjacent typo (--include-local-onlyy) still exits 2, so the codec is still strict.

The consent surface, which is where this needed the most scrutiny

A flag that widens disclosure is the one place a review should be least willing to take a claim on trust. Checked:

• The description on QUERY_OVERVIEW_SCHEMA (commands/query.js:98-105) is byte-identical to querySqlVerb's (query/verb.js:36-42), including the transcript-capture consequence, so the informed-consent language cannot drift between surfaces. Neither verb prints it in --help, so parity is exact; the wording that actually reaches a human is renderLocalOnlyNotice, shared by both.
• The wizard genuinely cannot pass it, verified in code rather than from the claim: wizard/index.js:178 calls firstLookRunnerFromCtx(opts.ctx, notices) with two arguments, so opts = {} and includeLocalOnly is false. hyp init --include-local-only is rejected outright.
• No implicit route exists. executeQuerySql reads the value from exactly one place (query/sql.js:222); there is no config key, no env var, and no preset. The two hardcoded includeLocalOnly: true call sites are pre-existing and untouched.
• query overview is a CLI command, not a verb, registered only at core_commands.js:113 with no MCP or tool projection, so an agent cannot invoke it with the flag through the tool surface.
• The bundled skills name hyp query overview but never the flag, so LLP 0105 #override's "the bundled skills never pass it" still holds. LLP 0135's new paragraph documents both the rationale and the wizard exclusion, landed in the same commit.

Your reasoning for excluding the wizard ("nothing in a setup step should quietly widen what a captured transcript can carry") is the right call and the code matches it.

The EPIPE fix

src/core/cli/stream_errors.js installs a stream-level 'error' listener on process.stdout and process.stderr at bin/hypaware.js:47-49, before any write. EPIPE is swallowed silently; any other code is reported once via a shared flag as hyp: output stream failed (<code>) and does not alter the exit code. That is the right shape: not a byte cap (which would have broken the exactness the fold-line counts need) and not a process-level uncaughtException (which would mask real errors on other commands), but scoped to exactly the two streams.

Verified: a 121,078 byte --json payload piped to head -1 exits 0 across three runs with no stack trace. No exit-code regression for non-EPIPE either: --json > /dev/full (ENOSPC) behaves identically before and after.

One honest caveat, since round 2 is what put this on your plate. The reviewer could not reproduce the pre-fix crash through the hyp binary in this environment: five runs at the same 121 KB, plus a variant whose reader exits immediately, all exited 0, because flushStream's own once('error') (cli/flush-streams.js:31) already covered the tail of the window. The underlying failure is real and timing-dependent (bare node -e "process.stdout.write('x'.repeat(400000))" | head -1 reproduces it here with Unhandled 'error' event), and the new test pins it with a real pipe rather than a throwing stub. So the fix is correct and worth having, but the window it closes is narrower than round 2's report implied. Round 2 measured a real crash; it appears the binary's existing flush handler was already absorbing most of that window.

Finding

1. NON-BLOCKING, carry-over, not a live defect. overview.js:318 declares let withheld = false (the runner-lifetime boolean sawWithholding() returns) and :348 declares const withheld = renderLocalOnlyNotice(...), a string, inside run's block scope. Verified harmless today: say is defined in the outer scope at :320, so its withheld = true assignment lexically resolves to the outer binding regardless of where it is called from, and nothing inside run reads withheld before :348. The hazard is only for a future edit: adding any read of withheld earlier in run hits the TDZ on the inner const and throws a ReferenceError inside the function that decides whether an LLP 0105 disclosure happens, which is the one place a silent failure is a privacy failure. A one-word rename (withheldLine) closes it. Do not hold the PR for this; a follow-up is fine.

Verification run

npm test: 2714 tests, 2706 pass, 7 fail, all in test/core/leave-command.test.js, the only failing file, matching this branch's baseline of 7 (the branch forks from an earlier point than master, which has 8). npm run typecheck clean. Targeted node --test across the six relevant test files: 134 pass, 0 fail. Smokes core_boot_noop, command_dispatch, package_bin_boot, and backfill_claude_fixture all ok; local_only_query_withhold and walkthrough_to_first_query fail identically on a git archive extract of origin/master, so they are pre-existing and environmental.

Conventions clean: zero added lines containing U+2014 across the whole PR diff, no statement-terminating semicolons in the new JS, no @typedef, no inline import() types, and all new @ref anchors resolve (LLP 0105 {#override}, LLP 0135 {#first-look} and {#disclosure}).

---

Three rounds in, this PR has been through more scrutiny than most and has come out the other side. The remaining item is a rename that changes no behavior. Whether that is worth one more push or a follow-up issue is your call, and so is landing this: neutral does not ready or merge contributor PRs.

[philcunliffe]

Round 3 reviewed at 64adc0d. Full record above.

Approving. The blocker is genuinely fixed, verified end to end against a real cache and the real CLI, in both directions and on the mixed path. You fixed it the harder and better way: supporting the flag rather than softening the advice, and you were right that the stderr notice named it on every withheld run, so the bug was broader than round 2 reported.

The consent surface got the hardest look, since a flag that widens disclosure is where a review should be least willing to take a claim on trust. It holds: the schema description is byte-identical to the query sql verb's, the wizard genuinely cannot pass the flag (verified in code, not from the claim), there is no config, env, or preset route, and query overview has no tool projection an agent could reach.

One correction owed on my side: round 2 measured a real EPIPE crash, but round 3 could not reproduce it through the hyp binary, because flushStream's existing once('error') was already covering most of that window. Your fix is correct and worth having, and the new test pins it with a real pipe, but the window it closes is narrower than I implied.

One non-blocking item remains, documented in the record: withheld is still shadowed at overview.js:318 vs :348. Verified harmless today, since say resolves to the outer binding and nothing reads it early. It is a latent TDZ hazard in the function that decides whether a disclosure happens, so a rename is worth doing sometime, but not worth another round.

Three rounds in, this has had more scrutiny than most PRs get. Merging is yours: neutral does not ready or merge contributor PRs.