Skip to content

feat: lessons as a typed artifact with follow-through tracking - #677

Closed
dripsmvcp wants to merge 4 commits into
vouchdev:testfrom
dripsmvcp:feat/lessons
Closed

feat: lessons as a typed artifact with follow-through tracking#677
dripsmvcp wants to merge 4 commits into
vouchdev:testfrom
dripsmvcp:feat/lessons

Conversation

@dripsmvcp

Copy link
Copy Markdown
Contributor

Summary

ClaimType already carried workflow and warning — the vocabulary of a procedural rule. what was missing is the loop that makes such a rule earn its place: nothing recorded whether a surfaced rule was actually followed, and nothing warned at propose time when a near-identical rule already existed. this adds ClaimType.LESSON and the two loops, in a new src/vouch/lessons.py.

Follow-through — observe, never edit

kb.mark_lesson_followed(claim_id, followed, context?) appends one observation to audit.log.jsonl (lesson.followed / lesson.not_followed, reversible=False) and edits nothing — not the lesson's text, status, or confidence. an observation about usage is not an edit to reviewed knowledge, so this needs no gate and has none: the module does not import proposals at all, and test_the_lessons_module_cannot_reach_the_review_gate asserts that. counts are derived from the log on read rather than stored, so there is no second copy to drift out of sync with the authoritative stream.

both verbs are added to effectiveness.GOOD_EVENTS / BAD_EVENTS, so the signal reaches the measurement it exists for (#426, already landed).

Repeat guard — warn, never merge

proposing a lesson that strongly overlaps an approved one attaches a loud repeat_lesson warning naming the existing rule. the proposal is still filed; nothing is merged or auto-approved.

it is deliberately lexical (token Jaccard over stopword-stripped tokens, threshold review.lesson_repeat_threshold, default 0.6) rather than only riding find_similar_on_propose: the embedding path needs the [embeddings] extra, and a repeat guard that silently disappears on a base install is exactly the situation where duplicate rules accumulate. it composes with the embedding warnings rather than replacing them, deduped by artifact id.

workflow and warning claims count as lessons too, so existing KBs get follow-through without re-typing their rules.

Test Plan

  • tests/test_lessons.py (19 tests): the observe-not-edit invariant (byte-compares the claim yaml before/after), the no-gate-path invariant, the guard warning without blocking, the unrelated-lesson and non-lesson negative cases, configurable threshold, follow_rate is None vs 0.0 for an unobserved rule, and the effectiveness handoff
  • test_capabilities four-site parity + test_hot_memory_universal_coverage green
  • full CI gate
$ .venv/bin/python -m pytest tests/ --ignore=tests/embeddings -p no:warnings
2697 passed, 1 skipped in 193.10s (0:03:13)

$ .venv/bin/python -m mypy src
Success: no issues found in 120 source files

$ .venv/bin/python -m ruff check src tests
All checks passed!

Closes #428

ClaimType already carried `workflow` and `warning` — the vocabulary of a
procedural rule ("run mypy before pushing", "never git add -A here"). what
was missing is the loop that makes such a rule earn its place. nothing
recorded whether a surfaced rule was actually followed, so a load-bearing
convention and a stale one nobody heeds looked identical; and nothing
warned at propose time when a near-identical rule already existed, so the
kb accumulated restatements of the same rule.

adds ClaimType.LESSON and two narrow loops in a new lessons module.

**follow-through.** `kb.mark_lesson_followed` appends one observation —
`lesson.followed` or `lesson.not_followed` — to audit.log.jsonl and edits
nothing: not the lesson's text, not its status, not its confidence. an
observation about usage is not an edit to reviewed knowledge, so the path
needs no gate and has none; the module does not import proposals at all,
and a test asserts it. the counts are derived from the log on read rather
than stored, so there is no second copy to drift. both verbs are
classified by eval.effectiveness, which is the measurement this signal
exists to feed.

**repeat guard.** proposing a lesson that strongly overlaps an approved
one attaches a loud `repeat_lesson` warning naming the existing rule. it
warns only — the proposal is still filed, nothing is merged. deliberately
lexical (token jaccard over stopword-stripped tokens, threshold
review.lesson_repeat_threshold, default 0.6): the embedding path needs
the [embeddings] extra, and a guard that silently disappears on a base
install is exactly the case where duplicates accumulate. it composes with
the existing embedding warnings rather than replacing them, deduped by
artifact id.

`workflow` and `warning` claims count as lessons, so existing kbs get
follow-through without re-typing their rules.

three methods across the four registration sites plus the hot-memory
coverage map. tests/test_lessons.py covers the observe-not-edit
invariant, the no-gate-path invariant, the guard's warn-without-blocking
behaviour, and the effectiveness handoff.

closes vouchdev#428
@dripsmvcp
dripsmvcp requested a review from plind-junior as a code owner July 30, 2026 20:17
@github-actions github-actions Bot added docs documentation, specs, examples, and repo guidance cli command line interface mcp mcp, jsonl, and http surfaces storage kb storage, migrations, schemas, and proposals retrieval context, search, synthesis, and evaluation tests tests and fixtures size: L 500-999 changed non-doc lines labels Jul 30, 2026
…faces

two ci gates were red.

`schemas/` is generated from the pydantic models and checked for drift on
every pr; adding `ClaimType.LESSON` changed `claim.schema.json` and the
regenerate step was missed. `python scripts/gen_schemas.py`, one enum
member added.

the diff-coverage gate wants 100% of changed python. the cli and mcp
bodies were asserted-registered but never called, and the config
fallbacks were untouched.

added: the cli listing with and without observations, the follow-rate
line, --include-retired against an archived rule, marking followed and
not-followed, the follow-through json, and a clean one-line error for an
unknown claim; the mcp tools round-tripping and raising the ValueError an
mcp host can render; every repeat_threshold fallback (unparseable,
non-mapping document, non-mapping review block, absent key, non-numeric
value) plus the configured value; and the repeat guard on empty text and
on excluding the claim being edited — a rule re-proposed as an edit of
itself must not warn that it duplicates itself.
@github-actions github-actions Bot added the schemas json schemas and generated schema assets label Jul 30, 2026
goals (vouchdev#427) landed on test while this branch was open, and both features
register a kb.* method at the same insertion points. every conflict was
additive; nothing from either side is dropped.

the two read handlers had been fused by the merge onto their shared
attach_hot_memory tail — split back into separate _h_list_goals /
_h_list_lessons and kb_list_goals / kb_list_lessons bodies, with
@mcp.tool() restored on kb_set_goal_status. hot_memory covers both
list methods, _CLI_MIRRORS carries all three irregular mirrors, and the
changelog keeps both entries.
test advanced to the agent registry (vouchdev#607) while the previous merge was
under test. that feature is cli and trust only — it registers no kb.*
method — so the only conflict was the shared unreleased changelog stanza,
and both entries are kept.
@plind-junior
plind-junior enabled auto-merge July 31, 2026 06:05
@plind-junior

Copy link
Copy Markdown
Member

the two invariants are the right ones and they are enforced the right way — an observation about usage is genuinely not an edit to reviewed knowledge, so keeping lessons.py free of any import of proposals and asserting that in test_the_lessons_module_cannot_reach_the_review_gate is a better guarantee than a code comment. deriving counts from the audit log on read rather than storing them is the same instinct and avoids the second copy that always drifts.

the repeat guard warning rather than merging is also correct, and doing it lexically so it survives a base install without the [embeddings] extra is the same reasoning #679 uses. no argument with either loop.

what i would want settled before merge is the VEP question, which this PR does not raise. it changes schemas/claim.schema.json — adding lesson to the claim type enum — plus models.py, plus three new kb.* methods across the four registration sites. every other PR in the queue right now carries an explicit "VEP: Not applicable — no object model, kb.* method, on-disk layout, bundle format, or audit-log shape change" section, and this one changes the object model and the method surface. by the standard the rest of the queue is being held to, this is the PR that needs one.

that is not an objection to the feature — it is that ClaimType.LESSON is written into committed yaml, so a KB that files lessons is not readable by an older vouch. that consequence deserves to be recorded somewhere durable rather than inferred from the schema diff.

one smaller thing worth confirming: treating existing workflow and warning claims as lessons is a nice migration story, but it means follow_rate starts reporting on claims whose authors never opted into being measured. None vs 0.0 for an unobserved rule is handled, so the numbers will not lie — but a reviewer seeing follow-through stats appear on rules they wrote before this landed may reasonably ask where they came from. a line in the changelog entry would cover it.

auto-merge was automatically disabled July 31, 2026 07:00

Pull request was closed

@plind-junior

Copy link
Copy Markdown
Member

The features has been deprecated

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

cli command line interface docs documentation, specs, examples, and repo guidance mcp mcp, jsonl, and http surfaces retrieval context, search, synthesis, and evaluation schemas json schemas and generated schema assets size: L 500-999 changed non-doc lines storage kb storage, migrations, schemas, and proposals tests tests and fixtures

Projects

None yet

Development

Successfully merging this pull request may close these issues.

feat: lessons as a typed artifact with follow-through tracking

2 participants