Uh oh!
There was an error while loading. Please reload this page.
A picker row can be hidden from the menu without ceasing to be a source (LLP 0200) - #674
A picker row can be hidden from the menu without ceasing to be a source (LLP 0200)#674bgmcmullen wants to merge 3 commits into
Conversation
…ce (LLP 0200) The `raw-anthropic` / `raw-openai` wizard rows cost every first-run user a question and earn it back for almost nobody. Beside a client row they compose byte-identical config (claude already contributes the `anthropic` upstream, codex `openai` + `chatgpt`, and the fold dedupes by name), so their checked state is cosmetic. Alone they capture nothing at all: projection is adapter-owned, `@hypaware/ai-gateway` registers no projector of its own, and the default `config` boot profile activates only what the config names - so a raw-only install proxies traffic correctly and writes zero rows. Add `hidden` to the `contributes.picker` contract rather than a row-id list in core, which would undo what LLP 0130 settled about who owns the picker table. `visiblePickerDescriptors` is the one display filter, used by the legacy prompt and by the wizard's gate and menu. Hiding is a display filter, never a catalog deletion. The owner map arming LLP 0192's fail-closed withholding is folded from picker descriptors, so deleting the two `contributes.picker` blocks would give `ai_gateway_messages` an empty owner list - which both withhold rules read as "never withhold", turning off a privacy guard under cover of a UI cleanup. The descriptors stay. Carry-through is narrower than it first looks. A hidden row rides through the selection only when the config on disk collects nothing the menu can show (a `--source raw-anthropic` install being reconfigured). Carrying on seed membership was tried and is wrong: seeding is derivative for these rows - `raw-openai` reads as configured whenever codex's `openai` upstream is present - so it resurrected that upstream the moment someone unchecked codex. The existing "unchecking a row still removes its gateway upstream" test caught it. What this does not fix: raw traffic still needs an adapter's projector to be recorded, and generic Anthropic-dialect traffic is still stamped `client_name: 'claude'` by the LLP 0115 fallback. Both are projector behaviour, independent of any picker row, and belong to LLP 0192's deferred attribution decision. Filed as #673. npm test: 3728 pass, 1 fail (`usage_policy.fold_tightened`, pre-existing on master). typecheck clean. Smokes green: walkthrough_picker_to_first_query, cli_bundled_plugins_activated, status_diagnostics. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
2af02e8 to
13f1680Compare
…stop offering the hidden rows Three review findings on the `hidden` picker field. 1. Carry-through could fire off a DETECTION seed. `promptPickSelection` read `carried` from `seed`, but `seed` is `initialSelection ?? config ?? detected`. LLP 0200 #carry-through scopes carry-through to "the config on disk collects nothing the menu can show"; off a detection seed it would instead compose a hidden row the user was never shown and cannot uncheck, which is what LLP 0011 #autodetect-vs-default forbids and what `pick.js`'s own @ref to it promises. Unreachable today only because no bundled hidden row declares a `detect` probe, and `hidden` is a kernel-contract field any plugin can set beside one. `seedIsChosen` now gates it, and a test drives detection directly to pin it. 2. README and CONTEXT.md still listed `raw-anthropic` / `raw-openai` among the rows the interactive walkthrough offers. Both now say the rows are `--source`-only and why. 3. `picker-disclosure.test.js` justified its raw-row assertions with "these two rows carry no adapter to disclose it anywhere else", which stopped being true when the menu (the only consumer of `summary`) stopped rendering them. Comment corrected; the assertions stay, because hiding is a display filter, not a deletion. Co-Authored-By: Claude <noreply@anthropic.com>
philcunliffe
commented
Aug 7, 2026
neutral review round: PR #674 @ |
…o pick Carry-through re-tested its own precondition on every pass. A re-entry seeds with the selection the previous pass confirmed (LLP 0191 #re-entry-seeding), which for a raw-only install holds the carried hidden row beside whatever visible rows the user just added. Asking "does the seed collect nothing the menu can show?" of that seed answers no, so the carried row was dropped: `back` then `enter` deleted the gateway upstream the install runs on, silently. Replaces the boolean `seedIsChosen` with a `SeedOrigin` the seed carries alongside it. `detected` never carries (unchanged), `config` carries only when the menu can show nothing the config collects (unchanged), and `selection` carries unconditionally: read-back never reaches that tier, so a hidden id is in a previous answer only because this rule put it there. LLP 0200 #carry-through records the re-entry case. Also pins the `hidden` manifest validation branch, which shipped untested. Co-Authored-By: Claude <noreply@anthropic.com>
philcunliffe
commented
Aug 7, 2026
neutral review round 2: PR #674 @ |
philcunliffe
left a comment
There was a problem hiding this comment.
Review capacity for this PR is spent (2 rounds, the ladder's cap), and the current head f6ae0c3 carries changes that have not themselves been reviewed. Handing the remainder back rather than deferring it - this is your PR, so residual items are yours to decide, not neutral's to defer into a follow-up issue.
What the two rounds did. Round 1 (head 13f1680) found 3 findings and fixed them. Round 2 (head 27bf6c0) found 2 more and fixed them - including a real bug that round 1's own fix had introduced: collapsing two seed tiers into one boolean meant stepping back into the picker lane dropped a carried hidden row, silently deleting the gateway upstream the install runs on. Reproduced end-to-end against the bundled catalog, and pinned by test/core/cli/wizard/pick.test.js:1102 (verified failing before the fix, passing after). Suite at the current head: 3752 passing, 0 failing; typecheck clean.
What needs your decision:
The LLP 0200 amendment. Round 2 edited an Accepted doc (
§carry-throughand§consequences) because its normative "in exactly one case" sentence needed a second case once re-entry was handled. The stated reasoning is that re-entry was never considered by the doc and its current behaviour contradicts LLP 0191, which is a different situation from re-litigating settled intent. That reasoning looks sound, but this repo's convention is that an Accepted LLP changes via a new LLP, so the call is yours: keep the amendment, or split it into a new doc. The code change stands on LLP 0191 either way.A fleet-locked hidden row vanishing from the defaults gate. Deliberately left unfixed across both rounds: LLP 0200 §hidden-rows settles the current behaviour, and it is unreachable against the bundled catalog. If "locked" should beat "hidden", that is a new LLP, not a code patch.
The current head is unreviewed.
f6ae0c3is neutral's own round-2 work. Given round 2 caught a defect in round 1's fix, that head deserves your eyes before merge rather than an automatic pass.
Both review records are in this thread with full file:line evidence. Nothing here is a known production defect - this is an exhausted-ladder handback, not a blocking finding.
Push to the branch or reply here and neutral will re-engage on its next tick.
…, LLP 0200 collision renumbered, cross-PR seams closed (#679) * A picker row can be hidden from the menu without ceasing to be a source (LLP 0200) The `raw-anthropic` / `raw-openai` wizard rows cost every first-run user a question and earn it back for almost nobody. Beside a client row they compose byte-identical config (claude already contributes the `anthropic` upstream, codex `openai` + `chatgpt`, and the fold dedupes by name), so their checked state is cosmetic. Alone they capture nothing at all: projection is adapter-owned, `@hypaware/ai-gateway` registers no projector of its own, and the default `config` boot profile activates only what the config names - so a raw-only install proxies traffic correctly and writes zero rows. Add `hidden` to the `contributes.picker` contract rather than a row-id list in core, which would undo what LLP 0130 settled about who owns the picker table. `visiblePickerDescriptors` is the one display filter, used by the legacy prompt and by the wizard's gate and menu. Hiding is a display filter, never a catalog deletion. The owner map arming LLP 0192's fail-closed withholding is folded from picker descriptors, so deleting the two `contributes.picker` blocks would give `ai_gateway_messages` an empty owner list - which both withhold rules read as "never withhold", turning off a privacy guard under cover of a UI cleanup. The descriptors stay. Carry-through is narrower than it first looks. A hidden row rides through the selection only when the config on disk collects nothing the menu can show (a `--source raw-anthropic` install being reconfigured). Carrying on seed membership was tried and is wrong: seeding is derivative for these rows - `raw-openai` reads as configured whenever codex's `openai` upstream is present - so it resurrected that upstream the moment someone unchecked codex. The existing "unchecking a row still removes its gateway upstream" test caught it. What this does not fix: raw traffic still needs an adapter's projector to be recorded, and generic Anthropic-dialect traffic is still stamped `client_name: 'claude'` by the LLP 0115 fallback. Both are projector behaviour, independent of any picker row, and belong to LLP 0192's deferred attribution decision. Filed as #673. npm test: 3728 pass, 1 fail (`usage_policy.fold_tightened`, pre-existing on master). typecheck clean. Smokes green: walkthrough_picker_to_first_query, cli_bundled_plugins_activated, status_diagnostics. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * New folders sync by default; one gate accepts the lot (LLP 0200/0201) Two changes to the same complaint: setup asks too much, and one of its questions comes back every session. LLP 0200 makes the per-folder classification ask (LLP 0106) opt-in. A machine-local preference decides whether an unclassified folder is asked about at session start, and its default is `sync` - the folder keeps the implicit default it already had and nobody is interrupted. The ask still exists for people who want it, chosen in the wizard's new-folder step or with `hyp policy folders ask`. This gates the question only: no class moves, `.hypignore` is untouched, and nothing new leaves a machine that was not already syncing by default. `hyp status` names the mode on every enrolled run so it is never a silent state, and the classification prompt itself now teaches its own off switch. The new-folder question is its own wizard step, after the per-adapter sync lane rather than a row on it: that lane answers "which adapters ship" and this one answers "what happens the next time I work somewhere new", and folding the second into the first let a checklist decide a policy silently. LLP 0201 adds an express gate before the question lanes: it lists what was found and offers "Record and sync all of these" or "Let me choose". Accepting skips the prompts, never the statements - each lane still narrates the block its gate would have shown, so the fast path prints the same rows in the same order, minus the keypresses. The rows come from the pick lane's own computation (hoisted to `resolvePickSeeding`), with detection run once per wizard run, so the two screens cannot disagree about what "all of these" means. With nothing detected and nothing locked there is nothing to accept, so no gate is shown. Also: the overwrite confirm is laid out instead of run together as one paragraph, same facts and order. npm test: 3765 pass, 1 fail (usage_policy.fold_tightened, identical on a clean checkout). npm run typecheck: clean. Smokes walkthrough_picker_to_first_query, status_diagnostics, cli_bundled_plugins_activated, daemon_foreground_start_stop: green. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * Review fixes: carry-through is scoped to a chosen seed, and the docs stop offering the hidden rows Three review findings on the `hidden` picker field. 1. Carry-through could fire off a DETECTION seed. `promptPickSelection` read `carried` from `seed`, but `seed` is `initialSelection ?? config ?? detected`. LLP 0200 #carry-through scopes carry-through to "the config on disk collects nothing the menu can show"; off a detection seed it would instead compose a hidden row the user was never shown and cannot uncheck, which is what LLP 0011 #autodetect-vs-default forbids and what `pick.js`'s own @ref to it promises. Unreachable today only because no bundled hidden row declares a `detect` probe, and `hidden` is a kernel-contract field any plugin can set beside one. `seedIsChosen` now gates it, and a test drives detection directly to pin it. 2. README and CONTEXT.md still listed `raw-anthropic` / `raw-openai` among the rows the interactive walkthrough offers. Both now say the rows are `--source`-only and why. 3. `picker-disclosure.test.js` justified its raw-row assertions with "these two rows carry no adapter to disclose it anywhere else", which stopped being true when the menu (the only consumer of `summary`) stopped rendering them. Comment corrected; the assertions stay, because hiding is a display filter, not a deletion. Co-Authored-By: Claude <noreply@anthropic.com> * Script detection in the wizard end-to-end so the express gate always shows CI failed on `runInitWizard end-to-end`: it read the real machine for detection, and the express gate (LLP 0201) is shown only when there is something to accept. A developer laptop with Claude installed gets the gate; a CI runner with no AI clients does not, so every scripted answer after the fork landed on the wrong prompt and the run finished on the team pathway it was supposed to step back out of. The rows themselves were never what the test was about ('all' picks whatever the menu offers); that there *are* rows is. So detection is injected like every other seam in that call, and the script is stable on any host. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * Trim two wizard screens: the overwrite confirm, and the asset install The overwrite confirm ran 44 words for a y/N. It still says the two things LLP 0183 #say-so requires - that the file is rewritten from the picks, and what carries over - in 27: the path already reads as "the config", and the enumeration compresses without losing its reassurance. The finale's client-asset step printed one line per copy, which on a two-client install is a dozen destination paths burying the step's one fact. It now reports counts per client, and counts them per client rather than summed: six skills copied to two clients is twelve copies, and neither client got twelve. The materializer keeps its per-copy output for `hyp skills install`, where the copies are the subject; the paths stay in the run summary and the skills.install span. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * Setup offers the first sync instead of only naming it (LLP 0200) LLP 0101's amendment already conceded the case: on an attended onboarding the hold blocks the demonstration that the product works, and the user had no way to say "I have seen enough". It minted `hyp sync` for that and stopped one step short - the wizard names the verb in the fifth line of a six-line narration, then hands the terminal to a client. A user with no privacy concern has to notice a sentence, remember a command, and find a terminal setup is giving away. So an enrolled attended run now asks: wait until the deadline, or send now. Waiting leads and is the default; the unwanted outcome of a mis-keyed wait is a few hours' latency, and of a mis-keyed send is permanent. The question is an offer, not the consent. "Send now" runs the real `hyp sync`, which prints the destination plan, escalates its warning because the window is open, and asks its own y/N - only that y clears the marker. LLP 0100 R2 and LLP 0101 #no-release hold verbatim: there is still exactly one release path, and this adds no second one. It spawns a child rather than running in process, and that is correctness, not convenience: `hyp init` boots all-available, which withholds @hypaware/central because a CLI boot must not acquire a server identity, so this process has no central sink handle. An in-process sync would render a plan missing the destination the release unblocks - the misleading artifact R2 requires the plan to prevent. Whether it sent is read back from the hold marker rather than from the exit code, which is 0 both for a release and for a user who read the list and said no. Position: narration, offer, first ask. The narration stays the last thing HypAware says about privacy; the first ask stays last on screen because it may take the terminal for good. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * Review fixes round 2: a carried hidden row survives stepping back into pick Carry-through re-tested its own precondition on every pass. A re-entry seeds with the selection the previous pass confirmed (LLP 0191 #re-entry-seeding), which for a raw-only install holds the carried hidden row beside whatever visible rows the user just added. Asking "does the seed collect nothing the menu can show?" of that seed answers no, so the carried row was dropped: `back` then `enter` deleted the gateway upstream the install runs on, silently. Replaces the boolean `seedIsChosen` with a `SeedOrigin` the seed carries alongside it. `detected` never carries (unchanged), `config` carries only when the menu can show nothing the config collects (unchanged), and `selection` carries unconditionally: read-back never reaches that tier, so a hidden id is in a previous answer only because this rule put it there. LLP 0200 #carry-through records the re-entry case. Also pins the `hidden` manifest validation branch, which shipped untested. Co-Authored-By: Claude <noreply@anthropic.com> * The express gate joins the forward chain but not the back chain Escape at the pick lane reached past the new gate to the fork, and escape at the sync lane landed on the gate instead of the picker: the question was inserted between the fork and the pick lane without being given a loop level, so both of its neighbours overshot by one screen. LLP 0191 #back-edges is that escape steps exactly one screen back. The gate now owns `atExpress` between `atFork` and `atPick`. Pick backs to it when the pass showed one and to the fork when it did not (LLP 0201 #no-default-no-accept), and a sync back re-runs the picker without re-asking it. Three tests pin the chain; the end-to-end walk gains the keypress the corrected edge needs. Also corrects three doc blocks left over from a draft where `ask` was the default. `src/core/usage-policy/types.d.ts` and `src/core/daemon/types.d.ts` ship in the published declaration build, so they were telling consumers the opposite of both the code and LLP 0200 #default about a privacy-relevant default; `usage-policy/index.js` said the same and attributed the write to the sync lane, which LLP 0200 #wizard explicitly rejects. Its export block had also been spliced between the client-sync comment and the export that comment describes. `hyp policy list` no longer drops "no machine-local entries (policy store: ...)" on a store that is empty while the folder mode is non-default; the mode is an addition to that line, not a replacement for it. Co-Authored-By: Claude <noreply@anthropic.com> * An unreadable hold re-read claimed the sync happened LLP 0200 #read-back settles the direction: "An unreadable re-read is treated as 'still held', because claiming a sync happened is the one wrong answer that cannot be corrected later." `readHold` said the same thing in its own docstring and then did the opposite - it returned `null` on a failed read, and `null` is the caller's word for "the marker is gone, it sent". So a state dir that would not resolve ended the run reporting `released: true`, logging `sync_now: released`, and skipping the line that restates the deadline: setup's last word on the subject was a release nobody observed. A throw from the `readDeadline` seam was worse in a second way - it escaped to the outer catch and returned `{ asked: false, reason: 'error' }` after the child had already run, so the run ended on silence with no statement of what still held. Both now return the deadline the step started from, which reads as "still held" and prints the restatement. Co-Authored-By: Claude <noreply@anthropic.com> * Escape at the new-folder lane traps a fleet-managed machine (LLP 0200/0201) The sync lane asks nothing when everything picked is fleet-locked, and nothing when the client store is corrupt: it states its outcome and returns. The new-folder lane behind it backed unconditionally into it, so escape re-ran the statement and re-asked the same question. On a fully managed machine that is a trap with no exit but ctrl+c. The lane now reports `noQuestion` on both non-prompting returns and the orchestrator steps past it to the picker, which is the last screen the user could answer. Same class one step earlier: the express gate lists the picker's own confirmed defaults, so a confirmed empty selection leaves it nothing to show. A back out of the re-entered picker landed on a gate that could not render and fell forward into the picker again. It now reaches the fork. Also: the pick lane re-derived `defaultRows` instead of taking the `resolvePickSeeding` value the express gate reads, which is the second derivation of "the defaults" LLP 0201 #gate says must not exist. Co-Authored-By: Claude <noreply@anthropic.com> * The sync read-back's production body was never run by a test Every test of `runWizardSyncNow` replaced the hold re-read with the `readDeadline` seam, so the body that actually decides the release claim - resolve the state dir from the environment, read the marker there - had no coverage at all. That is the body a wrong answer cannot be walked back in (LLP 0200 #read-back), and the blind spot is what let the earlier `catch { return null }` report a release nobody observed. Two tests drive it end to end against a real marker on disk with no seam: a spawn stub that clears the marker (a release) or leaves it (a declined plan). Proven to bite: dropping `opts.env` from the state-dir resolution - the drift that would make every run claim a release - fails only these two and leaves all eight pre-existing tests green. Co-Authored-By: Claude <noreply@anthropic.com> * Renumber 674's LLP 0200 to 0202 (folder-ask kept 0200; mechanical, LLP 0156) * Renumber 677's LLP 0200 to 0203 (mechanical, LLP 0156) * Restore 677's per-client asset counts dropped by the confirm-copy resolution * Add cross-PR regression test: express auto-accept honors hidden-row carry-through --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com> Co-authored-by: test <test@test.com> Co-authored-by: neutral <neutral@hyparam.com> Co-authored-by: neutral <neutral@hyperparam.app>
Drops the
raw-anthropic/raw-openairows from the setup wizard, via a newhiddenfield on the picker contract.Why
The two rows cost every first-run user a question and earn it back for almost nobody:
claudealready contributes theanthropicupstream,codexcontributesopenai+chatgpt, and the picker fold dedupes upstreams by name. LLP 0183 already named the consequence: the row reads back as configured whenever its upstream is, so its checked state is cosmetic.ai_gateway_messagesis adapter-owned -registerExchangeProjectoris called only by the claude, codex, and openclaw plugins, never by the gateway itself. The default boot profile isconfig(boot.js:571), which activates only what the config names. So a raw-only install proxies traffic to the provider correctly, countsaigw.exchange_bytes, warnsaigw.message_projection_skipped/no_projector_matchper exchange, and writes zero rows.Confirmed with
hyp init --yes --source raw-anthropic --dry-run: composes gateway + local-fs + format-parquet, no adapter.So the rows are redundant in the common case and non-functional in the case they exist for.
What changed
contributes.pickerrows may sethidden: true. A hidden row is absent from the interactive menu and the defaults gate, and is otherwise a picker source in every respect:--source <id>still composes it,configuredPickerSourcesstill reads it back, and its id keeps its opt-out/sync identity.hypaware-plugin-kernel-types.d.ts,src/core/types.d.ts-hidden?: booleansrc/core/manifest.js- rejects a non-booleansrc/core/plugin_catalog.js- carries it onto the descriptorai-gateway/hypaware.plugin.json- both raw rows set itwalkthrough.js-visiblePickerDescriptors(), the single display filterAdding a manifest field rather than a row-id list in core is deliberate: a hardcoded list would undo what LLP 0130 settled about who owns the picker table.
Two things worth reviewer attention
Hiding is a display filter, never a catalog deletion.
datasetOwnedSourceIdsFromCatalogfolds picker descriptors into the dataset-owner map that arms LLP 0192's fail-closed withholding. Deleting the twocontributes.pickerblocks - which looks like the same change - would giveai_gateway_messagesan empty owner list, and both withhold rules read that as "never withhold". A privacy guard would switch off under cover of a UI cleanup. That's written into thevisiblePickerDescriptorsdoc comment and into LLP 0200 so the next person doesn't finish the cleanup.Carry-through is narrower than it first looks. A hidden row rides through the selection only when the config on disk collects nothing the menu can show (a
--source raw-anthropicinstall being reconfigured). My first version carried any seeded hidden row, and the existingunchecking a row still removes its plugin and its gateway upstreamtest caught it: seeding is derivative for these rows -raw-openaireads as configured whenever codex'sopenaiupstream is present - so it resurrected that upstream the moment someone unchecked codex.Not fixed here
Raw traffic still needs an adapter's projector to be recorded, and generic Anthropic-dialect traffic is still stamped
client_name: 'claude'by the LLP 0115 fallback, polluting Claude-attributed queries and reports. Both are projector behaviour, independent of any picker row, and belong to LLP 0192's deferred attribution decision. Filed as #673.Docs
LLP 0200 records the decision;
Extended-by:forward-refs added to LLP 0130 and LLP 0012.Checks
CI green:
testandtypecheckon Node 22 and 24, plusduplicate-numbers.Locally:
npm run typecheckclean; smokes green forwalkthrough_picker_to_first_query,cli_bundled_plugins_activated,status_diagnostics. Two tests fail on this machine and pass in CI -resolve emits a hashed usage_policy.fold_tightened...and theclient_attach_idempotentsmoke - both of which also fail on an unmodifiedmastercheckout here, so they look environment-specific rather than related to this change.Four new tests cover hidden-row behaviour: absent from gate and menu, raw-only config survives a reconfigure, derivative seeding does not resurrect an unchecked upstream, and
--sourcestill composes.Note for #647
Rebased onto
masterafter #647, keeping its new raw-row copy ("starts a local gateway listener") alongside thehiddenflag - the manifest conflict was only that the two edits landed on adjacent lines. Worth flagging to that PR's author: a hidden row'ssummaryno longer renders anywhere, since the picker menu was its only consumer. The copy stays accurate and costs nothing, but the disclosure it adds is now unreachable for these two rows specifically.🤖 Generated with Claude Code