feat: add RunContract quality hints#111
Conversation
kapi-agent reviewVerdict: APPROVE Final approval summaryReview journey
Evidence
Notes
DecisionApproved for merge into |
There was a problem hiding this comment.
kapi-agent review
Verdict: APPROVE
Final approval summary
Review journey
This PR adds advisory RunContract quality hints for the existing RunContract projection. The semantic scope is narrow: introduce quality hint primitives, compute a coarse quality rollup, and keep completion authority tied to existing workflow status and evidence criteria. There was no prior kapi-agent review on this PR, so no review/fix ping-pong needed to be closed.
What changed
The main change is in src/domain/run-contract.ts:
- Replaces the placeholder quality status with
ready | attention | blocked | unknown. - Adds
ScoringHintwithid,level,reason, andevidenceBacked. - Computes hints for lifecycle blockers, missing evidence, satisfied evidence, pending decisions, and unresolved risks.
- Keeps
completionderived from existing completion criteria.
Tests in test/run-contract.test.ts now cover:
- Blocked quality when pending decisions exist.
- Ready quality when completion evidence is satisfied.
- Attention quality when unresolved risks remain despite completion.
Why this is correct
The implementation keeps the quality surface advisory. It does not change the existing completion calculation or make quality status authoritative for workflow completion. The hint ordering and rollup are straightforward: blocked dominates attention, attention dominates unknown, and otherwise the view is ready. The wording also explicitly discourages superficial score-passing and points back to evidence-backed workflow validation.
Evidence
- Verifier gate: PASS —
npm ci && npm run verifyexited 0. - Size gate: PASS — 87 changed lines, below the 200-line hard limit.
- Revision-explanation status: not required for this first kapi-agent review; PR body contains implementation and verification notes.
- Inspected files:
src/domain/run-contract.ts,test/run-contract.test.ts. - Additional search found no remaining references to the removed
"not-evaluated"quality status insrcortest.
Remaining risks and approval rationale
Remaining risk is low and mainly contract-evolution related: downstream consumers of RunContractView.quality.status must tolerate the new status vocabulary and hints field. Within this repository, no conflicting usages were found, tests cover the new behavior, and the change is small and scoped. Approval is justified.
Blocking issues
None.
Warnings / risks
src/domain/run-contract.ts: This changes the public RunContract quality status vocabulary from a placeholder to real statuses. Repository-internal usage appears safe, but external consumers may need to update if they depended on"not-evaluated".
Suggestions
- Consider documenting the quality status vocabulary wherever RunContract output is described for external consumers, if such docs exist outside this PR.
Looks good
- Quality hints remain advisory and do not alter completion authority.
- The rollup logic is simple and deterministic.
- Tests cover ready, blocked, and attention outcomes.
- The implementation stays generic and avoids product-/platform-specific gate names.
Verification notes
- Verifier gate: PASS —
npm ci && npm run verifyexited 0. - Size gate: PASS — 87 changed lines < 200.
- Revision-explanation status: not required; found in PR body anyway.
- PR evidence includes targeted RunContract tests, full verify, diff check, and grep for forbidden product-specific terms.
Engine: pi
Summary
Issue
Closes #100
Implementation notes
ScoringHintcarriesid,level, reason text, andevidenceBackedmetadata.ready,attention,blocked, orunknown.Verification
npm test -- test/run-contract.test.tsnpm run verifygit diff --checkgrep -RniE 'github|pullRequest|issueNumber|kapiAgent|discord|ragna' src/domain/run-contract.ts test/run-contract.test.tsreturned no matchesNotes
npm run verifystill reports existing non-failing quality budget warning:code_smells=44, target<=20, statuswarn.