refactor(vocab): shrink the interface, and derive the inertness verdict once - #337
Merged
Conversation
…go private
`vocab.zig` published eight names for what is one question per backend. Three
of them — `chars_per_token`, `target_tokens`, `near_tokens` — had no consumer
outside the file at all: they are tuning, not interface. `estimateTokens` had
two, both inside the very `switch (vocab.budget(list))` that had just computed
the same number, which is a shape that lets a caller classify one list and
print a token count taken from another.
Make the thresholds private and let `Budget` carry the estimate on the two arms
that print one:
pub const Budget = union(enum) { ok, near: usize, over: usize };
The `.ok` cell shows no number, so it carries none. A caller can no longer
obtain an estimate it will not display, forget one where the wording needs it,
or pair a verdict with a count from a different list.
Interface: 8 public decls to 4 — `Budget`, `budget`, `buildPrompt`,
`writeKeywordsJson`. No behaviour change; the thresholds and the ceil-divide
are untouched, and the crossing test still pins where ok/near/over fall.
The Vocabulary row and the dialog it opens both answered "is this list inert?"
and each derived it separately from the same two raw axes — `status_item` as
`selected == .openai and !keywords_capable`, `menu` as the `else` arm of an
if/else-if chain. Two shapes for one rule, in two files, agreeing only by
inspection. ADR-0011's defect 9 is exactly this, and a comment at the call site
already named the risk rather than removing it.
Name the verdict and derive it once:
pub const VocabularyReach = enum { local, both, local_only };
`SettingsView` carries the reach instead of the capability bool; `settingsView`
owns the one derivation from (backend, model). Both wording functions take it
and lose a parameter — the row checks `reach == .local_only`, the dialog gets
one arm per variant with no fallthrough. `.local` and `.local_only` both mean
"only Whisper is biased today" and differ only in whether OpenAI is selected
and ignoring the list, which is the whole reason one warns and the other does
not.
`vocabularyInfoText` moves to status_item.zig with its tests: ADR-0011 puts
every string the Status Item shows in that module, and this copy (added by
#333, after the ADR) was the one that landed on the far side. Only the wording
moves — menu.zig still calls it on demand when the dialog opens, because the
budget hint needs the live term list the Presentation deliberately does not
carry.
Behaviour is unchanged; the mapping is equivalent on all four (backend x
capability) cells and the exact row and dialog strings are asserted as before.
The withhold-on-unknown default survives as `.local_only`.
Tests: the reach truth table over seven model/backend cases, the default
withholding on an unanswered view, and that the row's suffix and the dialog's
"ignored on OpenAI" clause appear on exactly the same cell.
CONTEXT.md gains **Vocabulary** and **Vocabulary Reach** — the concept had two
spec documents and five commits behind it but no entry in 55 terms.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Candidate 4 of the 2026-07-29 architecture review. Two independent halves, one commit each.
1 —
vocab.zig: 8 public decls → 4Three of the eight had no consumer outside the file at all —
chars_per_token,target_tokens,near_tokens. They are tuning, not interface.estimateTokenshad two, both inside the veryswitch (vocab.budget(list))that had just computed the same number.The
.okcell prints no number, so it carries none. A caller can no longer obtain an estimate it will not display, forget one where the wording needs it, or — the real bug the old shape permitted — classify one list and print a token count taken from a different one.Remaining interface:
Budget,budget,buildPrompt,writeKeywordsJson.Not split into two modules, despite the review card's framing. The Whisper and OpenAI halves share no code and no constants, but nothing varies across the seam a split would create — one path each, no third coming. It would turn 130 lines into two modules of ~60 with one consumer apiece and buy no leverage.
2 — one reach verdict instead of two derivations
The row and the dialog both answered "is this list inert?", each deriving it from the same two axes in a different shape:
status_item.zigconst inert = selected == .openai and !keywords_capable;menu.zigelsearm of an if/else-if chainAgreeing only by inspection. ADR-0011's defect 9 is exactly this, and the call site carried a comment naming the risk rather than removing it.
SettingsViewcarries the verdict instead of the capability bool;settingsViewowns the one derivation. Both wording functions take it and lose a parameter — the row checksreach == .local_only, the dialog switches three ways with no fallthrough..localand.local_onlyboth mean "only Whisper is biased today" and differ only in whether OpenAI is selected and ignoring the list — which is precisely what the suffix exists to say.vocabularyInfoTextmoves tostatus_item.zigwith its tests. ADR-0011 puts every string the Status Item shows in that module; this copy (added by #333, after the ADR) was the one that landed on the far side of the seam. Only the wording moves —menu.zigstill calls it on demand, because the budget hint needs the live term list thePresentationdeliberately does not carry.Behaviour
Unchanged. The mapping is equivalent on all four
(backend × capability)cells:inert.local.both.local_onlyThe exact row and dialog strings are asserted as before, and the withhold-on-unknown default survives as
.local_only.Tests
SettingsView{}withholds rather than claiming OpenAI biasing667 tests pass.
zig build testand plainzig buildboth exit 0 — the latter matters, since lazy analysis leaves production arms unchecked in the test build.Docs
CONTEXT.md gains Vocabulary and Vocabulary Reach. The concept had two spec documents and five commits behind it but was not among the 55 terms — it appeared only inside other entries. No ADR: deriving a verdict once instead of twice has no live counter-case.
🤖 Generated with Claude Code