Skip to content

fix(cli): bridge two vocabularies, move a late warning, stop a contradictory PASS (#389) - #502

Merged
jasonssdev merged 5 commits into
mainfrom
fix/389-vocabulary-init-doctor
Aug 9, 2026
Merged

fix(cli): bridge two vocabularies, move a late warning, stop a contradictory PASS (#389)#502
jasonssdev merged 5 commits into
mainfrom
fix/389-vocabulary-init-doctor

Conversation

@jasonssdev

Copy link
Copy Markdown
Owner

Closes #389 — the last of its bullets. One needed no change; the rest are here.

Already fixed, verified rather than assumed

doctor's Ollama remediation already distinguishes "not installed" from "not running": it checks PATH and, when the binary is absent, points at the download page and mentions the macOS app instead of printing a serve command that does not exist on that machine. Traced to the commit that introduced it rather than taken from the issue text.

The three real ones

Vocabulary. reconcile reported "recorded 'X' as superseding 'Y'" while list shows deprecated in its STATUS column — two words for the action just performed and its effect, with nothing connecting them. The line now names the status the superseded concept will carry.

Placement. init's embedding stickiness note arrived after the choice it informs and below "Next: run openkos ingest" — past both moments it exists for. A note now precedes the picker; the concrete one naming the resolved tag sits above the call to action.

A green line under a red one. doctor printed [PASS] Backend host locality directly beneath [FAIL] Ollama reachable. It reports configuration, not liveness, which is correct — but the reader has no way to know the two answer different questions. It is now [SKIP] when Ollama is unreachable, following the same rule the model and embedding checks already use, with the configured host still in the detail so the fact survives.

What the reviews caught — three rounds, all of it mine

Round 1 fixed the placement and immediately created a new problem: an interactive run now printed the stickiness explanation twice, once before the picker and once after. Moving a warning earlier is worthless if the reader meets the same sentence again five lines down. A flag now selects: if the picker note carried the explanation, the later line only confirms which tag it applies to.

Round 1 also caught that a test added in that same commit was vacuous. It claimed to prove the note precedes a rendered embedding picker, but patching the installed-model probe to return an empty list makes that picker take its documented silent fallback — printing nothing at all. It passed for the same weak reason as its non-interactive sibling. It now reports an allowlisted model as installed and asserts the picker text actually appeared.

Rounds 2 and 3 were about prose telling lies: the doctor docstring still said the locality check has no skip branch, which round 1 had just made false; then my consolidation added a sentence saying the reasoning "is not repeated here" while leaving the repeat directly below it. Two lenses caught that one independently.

Verification

  • uv run pytest -q4032 passed, 1 skipped. uv run mypy . → Success, 179 files. ruff check / ruff format clean and convergent.
  • Four of the five new tests were driven RED→GREEN. The fifth was mutation-verified: forcing the locality check to skip unconditionally failed it and three pre-existing healthy-run tests.
  • The one-explanation-per-run claim has its own test, mutation-verified by making both branches print.
  • The corrected picker test now asserts the picker text rendered — otherwise it proves nothing about placement relative to it.

Native review

Five lineages, all approved with receipts, all zero blockers. The last one returned zero findings across all four lenses.

pre-pr denies this PR, same structural reason as #498 through #501: it measures the whole branch against origin/main while each receipt binds its own increment. receipt-binding / candidate-or-paths-mismatch, action explicit-maintainer-action. Every changed line was reviewed and approved.

…dictory PASS (#389)

Three of the four remaining bullets. The fourth needed no change: `doctor`'s
Ollama remediation already distinguishes "not installed" from "not running"
-- it checks PATH and, when the binary is absent, points at the download page
and the macOS app instead of a `serve` command that does not exist there.
Verified in the source and traced to the commit that added it.

`reconcile` reported "recorded 'X' as superseding 'Y'" while `list` shows
`deprecated` in its STATUS column, so the operator met two words for the
action just performed and its effect, with nothing connecting them. The line
now names the status the superseded concept will carry.

`init`'s embedding stickiness note arrived after the choice it informs AND
below "Next: run openkos ingest" -- past both moments it exists for. A note
now precedes the picker, and the concrete one naming the resolved tag was
moved above the call to action.

`doctor` printed a green `[PASS] Backend host locality` directly beneath
`[FAIL] Ollama reachable`. It reports configuration, not liveness, which is
correct -- but the reader has no way to know the two answer different
questions, so it read as a contradiction. It is now `[SKIP]` when Ollama is
unreachable, following the same rule the model and embedding checks already
use, with the configured host still named in the detail so the fact survives.

Reviewed under lineage `review-a592b741d66c0577`, high risk, canonical 4R,
approved with receipt, zero blockers.
…ng nothing (#389)

Folds in the three findings from this branch's own review.

Moving a warning earlier is worthless if the reader then meets the same
sentence again a few lines down, which is exactly what the previous commit
did on an interactive run. A flag now records whether the picker note already
carried the explanation: if it did, the later line only confirms which tag is
sticky; if it did not, the later line carries the explanation itself. All four
combinations of interactive-or-not and flag-or-not print it exactly once.

The sharpest finding was that a test added one commit ago was VACUOUS. It
claimed to prove the note precedes a rendered embedding picker, but patching
the installed-model probe to return an empty list makes that picker take its
documented SILENT fallback and print nothing -- so the test passed for the
same weaker reason as its non-interactive sibling. It now reports an
allowlisted model as installed and asserts the picker text actually appeared.

The header comment above `doctor`'s locality check still asserted it has no
skip branch, which the previous commit had just made false.

Reviewed under lineage `review-e278673103fe353a`, high risk, canonical 4R,
approved with receipt, zero blockers.
#389)

The doctor docstring described its backend-host-locality check as having no
`[SKIP]` branch and, unlike every other check, always passing. Two commits
ago that check gained a skip branch for the unreachable case, and the inline
comment beside the code was corrected then -- this copy was missed, so the
function docstring and the comment beside the code said opposite things about
the same check.

Verified against the implementation before rewriting: the status is literally
pass when reachable and skip otherwise, the result is never critical, the
exit-code gate fires only on a critical failure, and a non-empty detail is
produced on both branches.

Reviewed under lineage `review-160212498ddbc097`, high risk, canonical 4R,
approved with receipt, zero blockers.
…ocstrings (#389)

The reasoning for why doctor's backend-host-locality check skips when Ollama
is unreachable had ended up in three places: the command docstring and two
inline comment blocks. The docstring keeps it; the inline blocks say only
what is local to their position.

Two docstrings in the test module still described the old always-passes
behavior, which this branch made false three commits ago.

Reviewed under lineage `review-fa0fac70d7a87702`, high risk, canonical 4R,
approved with receipt, zero blockers.
…389)

The previous commit added a sentence saying the locality check's reasoning
is not repeated beside the code, and left the paragraph that repeated it
directly underneath. Two lenses caught it independently. That is the same
class of false claim this branch spent several rounds removing, so it goes
rather than drifts.

The block now states only what is local to its position: which client it
reuses, why it skips when Ollama is unreachable despite being able to answer
without it, and why it must never become a failure. Comment-only.

Reviewed under lineage `review-6eecbecf76705e3b`, high risk, canonical 4R,
approved with receipt, zero findings across all four lenses.
@jasonssdev
jasonssdev merged commit a4d1cea into main Aug 9, 2026
6 checks passed
@jasonssdev
jasonssdev deleted the fix/389-vocabulary-init-doctor branch August 9, 2026 06:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

surface polish — help text, list ordering, vocabulary, and doctor remediation

1 participant