feat(rating): evidence-anchored ratings — write the receipt before the class#85
Merged
Conversation
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Non-ignored classes (cited/shaped/misled/overlooked) now require a trimmed, non-empty, <=500-char evidence line, enforced by _validate_evidence() in both apply_session_ratings (whole-batch validation before the savepoint) and credit_one. The exposure-row stamp UPDATE now also writes evidence. Deviation: credit_one's evidence param defaults to None (not a required kwarg) as a compat shim, since the MCP memory.credit handler doesn't forward evidence yet (Task 3). This makes memory.credit cleanly ValueError on non-ignored classes until Task 3 wires it through, instead of TypeErroring. Adjusted the brief's test_credit_requires_evidence and three pre-existing tests/mcp/test_rating_tools.py credit-dispatch tests to match. Also added an optional "evidence" property to the apply_session_ratings wire schema in mcp/tools.py (additionalProperties:false was rejecting the key outright). Full details in .superpowers/sdd/task-2-report.md. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Live settings.json env (INJECT_MODE=deferred, embeddings backend) leaks into test runs; third occurrence of this leak class. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
memory.credit's inputSchema now requires evidence (maxLength 500) and forwards it to MemoryRatingService.credit_one, closing the gap where credit degraded with a clean ValueError because the handler dropped the arg. apply_session_ratings' per-item evidence property gains the maxLength cap and both descriptions state the evidence-first rule. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
… Optionals SqliteBackend.credit_one and the StorageBackend protocol never gained the evidence kwarg Task 2/3 added to MemoryRatingService, so callers going through the storage interface directly (not the MCP handler) hit the evidence requirement with no way to satisfy it. Threads evidence: str | None = None through protocol.py, sqlite.py (forwarded to the service), and agentcore.py (accepted for signature parity only - agentcore has no evidence storage and never validated it). Fixes test_credit_one_for_missing_memory_returns_skip, whose actual subject is the memory_missing skip path, by giving it a valid evidence string. Also narrows tool.description: str | None before "in" checks in test_rating_tools.py's schema-description tests (pyright reportOperatorIssue). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Update website/architecture.md (Self-rating loop section) and website/mcp-tools.md (Rating tools section) to describe the evidence-anchored rating requirement introduced earlier in this branch: - memory.credit and memory.apply_session_ratings require a one-line evidence statement for every non-ignored class (cited/shaped/misled/ overlooked); ignored is exempt and evidence is optional there. - MemoryRatingService validates the whole batch before writing any row, so apply_session_ratings rejects the entire batch loudly if any non-ignored entry is missing evidence. - Evidence is stored on session_memory_exposure.evidence (migration 0016, nullable; historical rows are untouched) and is audit-only -- no scoring reads it. - Documented the "Rating evidence" history (last 10) surfaced in the reflection/semantic drawers and the evidence column on /diagnostics. - Called out the naming distinction from reflections.evidence_count / evidence_count on semantic memories, which count synthesis source observations and are unrelated to rating evidence. No code changes: pyright (0 errors) and the full pytest suite (1627 passed, 22 skipped, 26 deselected) both ran clean with no stragglers needing fixes.
…ion prose Code review caught a stale memory_credit(kind, id, 'cited'|'shaped'|'misled') signature left over from before the evidence requirement, in the Contextual injection section describing the retrieve_relevant footer text. Actual footer (better_memory/services/relevant.py ~298-303): memory_credit(kind, id, class, evidence) - include a one-line evidence statement. Updated the sentence to match and mention the evidence line. Grepped architecture.md again for memory_credit -- no other stale instance.
There was a problem hiding this comment.
🔴 Claude BugBot Analysis
Found 1 potential bug in this PR.
medium: 1
One genuine bug found: the memory.credit handler forwards the new evidence argument to the local sqlite rating service but forgets to forward it on the remote/AgentCore backend branch, silently dropping caller-supplied evidence for that backend.
Addresses PR #85 review: the agentcore branch of the credit handler silently discarded caller-supplied evidence. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
There was a problem hiding this comment.
🟢 Claude BugBot Analysis
Reviewed all production code changes in the diff (migration 0016, MemoryRatingService evidence validation, MCP tool schemas/handlers, storage backends, UI queries/routes/templates/CSS). The evidence-requirement feature is implemented consistently end-to-end with no logic errors, null dereferences, injection risks, or off-by-one issues found.
No bugs were detected in this PR.
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.
Summary
Implements §4 of the retrieval-quality design (evidence-anchored ratings) with the post-PR-D deltas.
Problem: rating variance is the noise floor under every ranking signal — byte-identical memory sets were rated
shapedin one session andignoredin another. Nothing anchored a non-ignored rating to anything observable.Mechanism — evidence-first: the rater writes ONE line of evidence (what the memory changed, or a quote) before choosing a class; nothing to point at ⇒
ignored. Enforced server-side:cited/shaped/misled/overlookedwithout a non-empty (≤500 chars, trimmed)evidencestring reject the whole batch pre-savepoint, loudly.evidence TEXTonsession_memory_exposure(additive; historical rows untouched)._validate_evidencein the existing validate-before-savepoint pass; evidence stamped in the same UPDATE asclassification.memory.creditrequiresevidencein-schema;apply_session_ratingsitems carry it optional-at-wire with service validation.ignoredpost-evidence)..chip.outcome-no_outcome).credit_oneevidence threading; test Optionals narrowed.Validation
Deploy
Merge → checkout main + pull. Migration 0016 applies on next MCP server start (may already be applied — editable working tree). The user-level skill junction picks up the SKILL.md rewrite automatically. First live RATE_MEMORIES sweep after restart exercises the contract.
🤖 Generated with Claude Code