feat: lessons as a typed artifact with follow-through tracking (#428)#472
feat: lessons as a typed artifact with follow-through tracking (#428)#472Tet-9 wants to merge 1 commit into
Conversation
WalkthroughAdds a new ChangesLesson Follow-Through Feature
Estimated code review effort: 3 (Moderate) | ~25 minutes Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
2d075f3 to
f1b51cc
Compare
|
@plind-junior , on for review |
f1b51cc to
3094bcf
Compare
|
@plind-junior , if you may review this now |
3094bcf to
798e412
Compare
|
@plind-junior , if you may review this |
|
@coderabbitai review |
✅ Action performedReview finished.
|
…dev#428) Adds ClaimType.LESSON and kb.mark_lesson_followed -- an append-only observation of whether a surfaced lesson was actually followed in a given turn. Never edits the claim; appends a lesson.followed audit event with followed, optional context, and claim_type. Registered across all four surfaces: CLI (mark-lesson-followed), MCP (kb_mark_lesson_followed), JSONL (kb.mark_lesson_followed), and capabilities.METHODS. Added to the _STANDARD MCP tool profile alongside kb.confirm, its closest existing analogue. Lessons resurface through the normal retrieval path with no special-casing anywhere in context.py -- verified directly, not assumed. The propose-time repeat guard (vouchdev#147) comes for free: propose_claim already runs find_similar_on_propose for every claim type regardless, so no new code was needed for that acceptance criterion. Not restricted to ClaimType.LESSON: mark_lesson_followed accepts any claim id, matching the issue's own proposed surface ("either a new ClaimType.LESSON or a flag on workflow/warning claims"). Deliberately does not build the "effectiveness computation" system vouchdev#428 names as a downstream consumer (vouchdev#2) -- that reference resolves to an unrelated PR and no such system exists anywhere in this codebase under any name (verified via grep). These events are shaped for such a consumer to exist later; inventing an unspecified analytics system was out of scope. tests/test_lessons.py: 13 tests covering the claim type, retrieval resurfacing, the observe-not-edit invariant, multiple observations, missing- claim handling, the not-restricted-to-LESSON case, registration in capabilities/JSONL/MCP, and the repeat-guard warning.
798e412 to
382d1f9
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@tests/test_lessons.py`:
- Around line 161-174: Add a failure-path test alongside
test_jsonl_mark_lesson_followed_end_to_end that calls kb.mark_lesson_followed
with a missing claim_id and asserts the JSONL response preserves the request id,
sets ok to false, and includes an error field.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 1161ef72-c121-4778-a019-9f52a647f07f
📒 Files selected for processing (12)
CHANGELOG.mdschemas/claim.schema.jsonsrc/vouch/capabilities.pysrc/vouch/cli.pysrc/vouch/hot_memory.pysrc/vouch/jsonl_server.pysrc/vouch/lifecycle.pysrc/vouch/mcp_profiles.pysrc/vouch/models.pysrc/vouch/server.pytests/test_lessons.pytests/test_propose_similarity.py
| def test_jsonl_mark_lesson_followed_end_to_end(store: KBStore, monkeypatch) -> None: | ||
| monkeypatch.chdir(store.root) | ||
| _lesson(store) | ||
| resp = handle_request( | ||
| { | ||
| "id": "r1", | ||
| "method": "kb.mark_lesson_followed", | ||
| "params": {"claim_id": "lesson-mypy", "followed": True}, | ||
| } | ||
| ) | ||
| assert resp["ok"] is True, resp | ||
| assert resp["result"]["id"] == "lesson-mypy" | ||
| assert resp["result"]["followed"] is True | ||
|
|
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Add a test for the JSONL envelope failure shape.
As per path instructions and coding guidelines, every new kb.* method must include a test asserting the JSONL envelope shape for both success ({"id", "ok", "result"}) and failure ({"id", "ok": false, "error"}). This file currently only tests the success path for kb.mark_lesson_followed over JSONL.
💡 Proposed failure test
def test_jsonl_mark_lesson_followed_failure(store: KBStore, monkeypatch) -> None:
monkeypatch.chdir(store.root)
resp = handle_request(
{
"id": "r2",
"method": "kb.mark_lesson_followed",
"params": {"claim_id": "missing-claim", "followed": True},
}
)
assert resp["ok"] is False
assert resp["id"] == "r2"
assert "error" in resp🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@tests/test_lessons.py` around lines 161 - 174, Add a failure-path test
alongside test_jsonl_mark_lesson_followed_end_to_end that calls
kb.mark_lesson_followed with a missing claim_id and asserts the JSONL response
preserves the request id, sets ok to false, and includes an error field.
Sources: Coding guidelines, Path instructions
Description
Fixes #428
What's included
ClaimType.LESSON — resurfaces through the normal retrieval path like any claim, no special-casing
kb.mark_lesson_followed(claim_id, followed, context?) — append-only observation appended to audit.log.jsonl as a lesson.followed event; never edits the claim's text/status/any field
Registered across all four surfaces: CLI (vouch mark-lesson-followed), MCP tool, JSONL handler, capabilities.METHODS, and the _STANDARD tool profile
Repeat guard: no new code needed — propose_claim already runs find_similar_on_propose (#147) for every claim type, so proposing a near-duplicate lesson already surfaces a non-blocking similar_approved/similar_pending warning
Deliberately out of scope
The issue names an "effectiveness computation" system (cross-referenced as #2) as a downstream consumer of these events. That reference resolves to an unrelated PR, and no such system exists anywhere in this codebase under any name (verified via grep across src/vouch/). Rather than invent an unspecified analytics/scoring system, this PR ships events shaped for such a consumer (followed, context, claim_type fields) without building the consumer itself.
Design note
mark_lesson_followed isn't restricted to ClaimType.LESSON — it accepts any claim id, matching the issue's own proposed surface ("either a new ClaimType.LESSON or a flag on workflow/warning claims"). Covered by a dedicated test.
Testing
tests/test_lessons.py: 13 tests — claim type, retrieval resurfacing, observe-not-edit invariant, repeated observations, missing-claim error, not-restricted-to-LESSON, registration across capabilities/JSONL/MCP, and the repeat-guard warning
Full suite: ruff check / mypy clean, existing test_capabilities.py and similarity-warning tests (test_propose_similarity.py) unaffected
CHANGELOG.md updated under [Unreleased]
Summary by CodeRabbit
New Features
lessonclaims.Documentation
vouch-kbcommand alias.