Skip to content

fix: require structured kapi-agent revision comments#125

Merged
devkade merged 1 commit into
devfrom
docs/revision-comment-rules
May 15, 2026
Merged

fix: require structured kapi-agent revision comments#125
devkade merged 1 commit into
devfrom
docs/revision-comment-rules

Conversation

@devkade
Copy link
Copy Markdown
Owner

@devkade devkade commented May 15, 2026

Summary

  • require stale/non-approving kapi-agent re-review requests to include same-comment @kapi-agent review plus current-head revision evidence
  • enforce required revision headings, current head SHA, non-speculative verification, and no bot-verdict impersonation in kapi-review
  • document the standard template in README and the approval-gate guide

Verification

  • npm run verify passed
  • git diff --check passed via the verification sequence

Notes

  • This PR intentionally keeps formal approval enforcement separate from author revision evidence. Revision comments can request review and explain changes, but they cannot become approval evidence.

@devkade
Copy link
Copy Markdown
Owner Author

devkade commented May 15, 2026

@kapi-agent review

Revision explanation for current head 7395d7979bc0236484d6d3a7a9b2c087f4b24b67:

What changed:

  • Enforced same-comment revision evidence in kapi-review for stale or non-approving prior kapi-agent reviews.
  • Documented the required revision-comment template and rules.

Why this closes the prior feedback:

  • Prior blocking issue: revision re-review requests could be split across trigger-only comments and separate explanations, making stale/current-head evidence ambiguous.
  • Resolution: the gate now requires trigger, current head SHA, structured change rationale, and verification in one comment when revision evidence is required.

Verification:

  • npm run verify passed.
  • git diff --check passed via the verification sequence.

Copy link
Copy Markdown

@kapi-agent kapi-agent Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

kapi-agent review

Verdict: REQUEST_CHANGES

Blocking issues

  • src/cli/kapi-review-cli.ts:17-24 does not enforce the documented “non-approving review” case unless the prior review is on an older head. The new docs say that after any stale or non-approving kapi-agent review, re-review must include same-comment revision evidence, but the gate is currently:
    • previousReviewOnOlderHead && !hasRevisionExplanation(...)
    • This means a prior REQUEST_CHANGES or COMMENT on the current head can be followed by an approval-shaped body with no revision explanation and still pass the revision_explanation gate.
    • That weakens the workflow contract this PR is trying to document/enforce. Please make the required condition match the docs, e.g. require revision explanation when the prior review is stale or when the latest prior state is non-approving, and add a regression test for same-head REQUEST_CHANGES/COMMENT without a revision explanation.

Warnings / risks

  • docs/kapi-agent-approval-gate.md:47 sets a stricter contract than the current implementation. Until the code and tests match that sentence, future maintainers may believe the gate protects same-head non-approving re-review requests when it does not.

Suggestions

  • Add explicit tests for:
    • prior REQUEST_CHANGES on the same headSha with no revision explanation => fail;
    • prior COMMENT on the same headSha with no revision explanation => fail;
    • prior APPROVE on the same headSha => no revision explanation required, if that is intended.

Looks good

  • The structured revision comment parser is substantially stronger than the previous keyword heuristic.
  • Requiring the trigger, current head SHA, named sections, bullets, non-speculative verification, and no bot-verdict impersonation is directionally sound.
  • The docs and README updates clearly explain the intended author workflow.

Verification notes

  • Verifier gate status: PASS — provided evidence says npm ci && npm run verify exited 0.
  • Size gate status: PASS — 101 changed lines, under the 200-line limit.
  • Revision-explanation status: not required for this first kapi-agent review; found evidence is present.
  • Local inspection covered src/cli/kapi-review-cli.ts, test/kapi-review-cli.test.ts, docs/kapi-agent-approval-gate.md, and the README diff.

Engine: pi

@devkade devkade force-pushed the docs/revision-comment-rules branch 2 times, most recently from 4dc95d2 to 053eecb Compare May 15, 2026 07:34
@devkade
Copy link
Copy Markdown
Owner Author

devkade commented May 15, 2026

@kapi-agent review

Revision explanation for current head 053eecb30ba252dc9f68cf7ad29be73f46c5ebec:

What changed:

  • Matched implementation to the documented non-approving review rule.
  • Added regression coverage for same-head prior REQUEST_CHANGES and COMMENT without revision evidence.
  • Removed the accidental executable mode churn from the TypeScript CLI file.

Why this closes the prior feedback:

  • Prior blocking issue: same-head REQUEST_CHANGES or COMMENT could be followed by an approval-shaped review body without revision explanation.
  • Resolution: revision evidence is now required when a prior review is stale or when the latest prior state is non-approving, even on the same head.

Verification:

  • npm test -- test/kapi-review-cli.test.ts passed.
  • npm run verify passed.
  • git diff --check passed.

Copy link
Copy Markdown

@kapi-agent kapi-agent Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

kapi-agent review

Verdict: REQUEST_CHANGES

Blocking issues

  • Revision explanation gate failed. This PR was updated after a prior kapi-agent REQUEST_CHANGES review, but there is no valid author explanation for the current head SHA explaining what changed and why. The existing author comment references an older head, so it is stale. Please add a new same-comment re-review request for the current head with:
    • @kapi-agent review
    • Revision explanation for current head <current SHA>
    • What changed
    • Why this closes the prior feedback
    • Verification

Warnings / risks

  • No additional code-level blocker found in the inspected changes. The current implementation appears to address the prior same-head non-approving review gap, but the workflow contract requires current-head revision evidence before approval can be considered.

Suggestions

  • Consider keeping the revision comment template in the PR body or latest author comment whenever pushing follow-up commits, since this PR is specifically enforcing that workflow.

Looks good

  • src/cli/kapi-review-cli.ts now requires revision evidence when a prior review is stale or when the prior kapi-agent review state is non-approving (REQUEST_CHANGES / COMMENT).
  • The revision explanation parser now checks for same-comment trigger, matching head SHA, required structured sections, bullets, bot-verdict impersonation, and speculative verification language.
  • test/kapi-review-cli.test.ts includes regression coverage for stale reviews and same-head non-approving reviews without revision evidence.

Verification notes

  • Verification gate status: FAIL overall because the revision explanation gate failed.
  • Size gate status: PASS — 114 changed lines, under the 200-line limit.
  • Revision-explanation status: FAIL — required now, not found for the current head.
  • CI/local evidence from provided verifier: npm ci && npm run verify exited 0.

Engine: pi

Copy link
Copy Markdown

@kapi-agent kapi-agent Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

kapi-agent review

Verdict: REQUEST_CHANGES

Blocking issues

  • Revision explanation gate failed. This PR was updated after a prior kapi-agent REQUEST_CHANGES review, but there is no author revision explanation for the current head 053eecb30ba252dc9f68cf7ad29be73f46c5ebec. Please add a same-comment re-review request explaining what changed and why for this exact head before re-review.

Warnings / risks

  • The previous revision explanation references old head 7395d7979bc0236484d6d3a7a9b2c087f4b24b67, so it cannot establish what changed in the current synchronized revision.

Suggestions

  • Use the documented template from docs/kapi-agent-approval-gate.md:
    • @kapi-agent review
    • Revision explanation for current head \053eecb`:`
    • What changed
    • Why this closes the prior feedback
    • Verification

Looks good

  • The implementation appears to address the prior substantive blocker: src/cli/kapi-review-cli.ts now requires revision evidence for stale prior reviews and same-head non-approving prior reviews.
  • The new tests in test/kapi-review-cli.test.ts cover stale approval/comment cases, same-head REQUEST_CHANGES/COMMENT, head SHA mismatch, missing trigger, malformed headings, speculative verification, and bot-verdict impersonation.
  • The documentation additions in README.md and docs/kapi-agent-approval-gate.md match the intended same-comment revision-evidence workflow.

Verification notes

  • Verification gate status: FAIL, because the revision explanation gate failed.
  • Size gate status: PASS — 114 changed lines, under the 200-line limit.
  • Revision-explanation status: FAIL — required now, not found for current head.
  • Local verifier evidence: PASS — npm ci && npm run verify exited 0.
  • Inspected files: src/cli/kapi-review-cli.ts, test/kapi-review-cli.test.ts, README.md, and docs/kapi-agent-approval-gate.md.

Engine: pi

@devkade
Copy link
Copy Markdown
Owner Author

devkade commented May 15, 2026

@kapi-agent review

Revision explanation for current head 053eecb30ba252dc9f68cf7ad29be73f46c5ebec:

What changed:

  • Matched implementation to the documented non-approving review rule.
  • Added regression coverage for same-head prior REQUEST_CHANGES and COMMENT without revision evidence.
  • Confirmed the PR diff no longer contains executable mode churn.

Why this closes the prior feedback:

  • Prior blocking issue: the latest kapi-agent review said the current head lacked valid revision explanation after REQUEST_CHANGES.
  • Resolution: this comment is a fresh same-comment re-review request for exact current head 053eecb30ba252dc9f68cf7ad29be73f46c5ebec, and the code now requires revision evidence for stale or non-approving prior reviews.

Verification:

  • npm test -- test/kapi-review-cli.test.ts passed.
  • npm run verify passed.
  • git diff --check passed.

@devkade
Copy link
Copy Markdown
Owner Author

devkade commented May 15, 2026

@kapi-agent review

Revision explanation for current head 78a2f5f9179549aff28ca45240afdd4cac1ef326:

What changed:

  • Refreshed the PR head after the latest kapi-agent REQUEST_CHANGES review so the required current-head revision evidence is discoverable during the next pull_request review pass.
  • Kept the actual implementation unchanged: revision evidence is required for stale reviews and same-head REQUEST_CHANGES/COMMENT reviews.
  • Kept the regression tests and documentation updates from the prior head.

Why this closes the prior feedback:

  • Prior blocking issue: kapi-agent did not detect valid revision explanation for current head 053eecb30ba252dc9f68cf7ad29be73f46c5ebec after REQUEST_CHANGES.
  • Resolution: this comment is prepared for the new current head 78a2f5f9179549aff28ca45240afdd4cac1ef326 before pushing that head, so the triggered review pass can see matching same-comment revision evidence.

Verification:

  • npm test -- test/kapi-review-cli.test.ts passed.
  • npm run verify passed.
  • git diff --check passed.

@devkade devkade force-pushed the docs/revision-comment-rules branch from 053eecb to 78a2f5f Compare May 15, 2026 07:39
@kapi-agent
Copy link
Copy Markdown

kapi-agent Bot commented May 15, 2026

kapi-agent review

Verdict: APPROVE

Final approval summary

Review journey

  • Original goal: require structured, current-head author revision evidence before kapi-agent can approve after stale or non-approving prior reviews.
  • The semantic scope is limited to the review CLI gate, its tests, and the documented revision-comment contract.
  • Prior kapi-agent reviews requested changes because same-head REQUEST_CHANGES/COMMENT reviews could be followed by approval without revision explanation, and then because current-head revision evidence was missing.
  • The latest author comment provides a current-head explanation for 78a2f5f9179549aff28ca45240afdd4cac1ef326.

What changed

  • src/cli/kapi-review-cli.ts now requires revision explanation when a prior kapi-agent review is stale or non-approving.
  • Revision explanations must include same-comment @kapi-agent review, matching current head SHA, required headings, bullets, non-speculative verification, and no bot-verdict impersonation.
  • test/kapi-review-cli.test.ts adds regression coverage for stale reviews and same-head non-approving reviews.
  • docs/kapi-agent-approval-gate.md and README.md document the required revision-comment template and rules.

Why this is correct

  • The implementation now matches the documented workflow contract: stale approval, stale comment, same-head REQUEST_CHANGES, and same-head COMMENT all require valid revision evidence.
  • The gate rejects stale SHA explanations, split trigger/explanation comments, missing required sections, speculative verification wording, and approval-shaped author impersonation.
  • This closes the previous blocker around same-head non-approving reviews bypassing revision evidence.

Evidence

  • Verifier gate: PASS — npm ci && npm run verify exited 0.
  • Size gate: PASS — 114 changed lines, under the 200-line limit.
  • Revision-explanation status: PASS — current-head explanation found for 78a2f5f9179549aff28ca45240afdd4cac1ef326.
  • Inspected files: src/cli/kapi-review-cli.ts, test/kapi-review-cli.test.ts, docs/kapi-agent-approval-gate.md, README.md.
  • Local inspection confirmed the current head contains the intended implementation and regression tests.

Remaining risks and approval rationale

  • Remaining risks: low. The revision parser is intentionally template-based and may reject some natural-language variants, but that is consistent with this PR’s stricter workflow contract.
  • Approval is justified because all gates pass, the PR is below the size limit, the current revision explains what changed and why, the previous blocking feedback is addressed, and no blocking correctness or safety issue remains.

Blocking issues

  • none.

Warnings / risks

  • The revision-comment validator is strict about exact headings and bullet presence in src/cli/kapi-review-cli.ts. This is acceptable for a workflow gate, but future wording changes in docs must stay synchronized with the parser and tests.
  • Test output includes Node [DEP0205] module.register() deprecation warnings from the test runtime; not caused by this PR’s logic.

Suggestions

  • Consider adding a small helper or shared constant for the revision-comment headings if future PRs extend this template further.

Looks good

  • The prior same-head non-approving review gap is closed.
  • The docs now clearly distinguish formal approval enforcement from author revision evidence.
  • Regression tests cover the important stale/current-head and malformed explanation cases.

Verification notes

  • Verifier gate: PASS — npm ci && npm run verify exited 0.
  • Size gate: PASS — 114 changed lines < 200.
  • Revision-explanation status: PASS — required current-head explanation found.
  • PR evidence includes passing full verification and targeted regression coverage for kapi-review.

Engine: pi


kapi-agent request lifecycle:

  • Status: COMPLETED
  • Head: 053eecb30ba2
  • Formal review event: APPROVE
  • Review API status: 422
  • Required check API status: 201

Copy link
Copy Markdown

@kapi-agent kapi-agent Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

kapi-agent review

Verdict: APPROVE

Final approval summary

Review journey

  • Original goal: enforce structured current-head revision evidence before kapi-agent can approve after stale or non-approving prior reviews.
  • The semantic scope is limited to the review CLI gate, regression tests, and documentation for revision-comment rules.
  • Prior kapi-agent reviews requested changes because same-head REQUEST_CHANGES/COMMENT reviews could be followed by approval without revision evidence, and later because the current head lacked a detected revision explanation.
  • The current revision includes a valid author explanation for head 78a2f5f9179549aff28ca45240afdd4cac1ef326.

What changed

  • src/cli/kapi-review-cli.ts now requires revision explanation when the prior review is stale or when the latest prior kapi-agent state is REQUEST_CHANGES or COMMENT.
  • Revision evidence must include same-comment @kapi-agent review, matching current head SHA, required headings, bullets, non-speculative verification, and no bot-verdict impersonation.
  • test/kapi-review-cli.test.ts adds regression coverage for stale reviews, same-head non-approving reviews, malformed explanations, and CLI persistence.
  • README.md and docs/kapi-agent-approval-gate.md document the required re-review template.

Why this is correct

  • The implementation now matches the documented workflow contract: stale approvals/comments and same-head non-approving reviews cannot be converted into approval unless the author provides structured current-head evidence.
  • The added tests cover the previously blocking gap for same-head REQUEST_CHANGES/COMMENT without revision evidence.
  • The explanation format rejects stale SHA references, split trigger/explanation comments, approval-shaped author comments, missing required sections, and speculative verification wording.

Evidence

  • Verifier gate: PASS — npm ci && npm run verify exited 0.
  • Size gate: PASS — 114 changed lines, below the 200-line limit.
  • Revision-explanation status: PASS — valid current-head explanation found.
  • Inspected files: src/cli/kapi-review-cli.ts, test/kapi-review-cli.test.ts, README.md, and docs/kapi-agent-approval-gate.md.

Remaining risks and approval rationale

  • Remaining risks: low; the gate depends on the caller passing the correct latest prior review state/head and the correct same-comment body as revisionExplanation.
  • Approval is justified because all gates pass, the PR is small and scoped, the prior blocking feedback is addressed, and no blocking correctness, safety, or workflow-contract issue remains.

Blocking issues

  • none.

Warnings / risks

  • The revision gate is only as reliable as the surrounding GitHub integration that selects the latest prior kapi-agent review and passes the correct author comment body into --revision-explanation-file.

Suggestions

  • Consider adding an integration-level test or fixture around comment selection if that logic lives outside this CLI, so the “same-comment” guarantee is verified end-to-end.

Looks good

  • The new revisionExplanationRequired condition closes the same-head non-approving review gap.
  • The structured revision parser checks current head SHA, required headings, section bullets, bot-verdict impersonation, and speculative verification.
  • The documentation now clearly tells authors how to request re-review after stale or non-approving kapi-agent feedback.

Verification notes

  • Verifier gate: PASS — npm ci && npm run verify exited 0.
  • Size gate: PASS — 114 changed lines < 200.
  • Revision-explanation status: PASS — required and found for current head.
  • Local inspection found the implementation and tests aligned with the prior review feedback.

Engine: pi

@devkade devkade merged commit 64610d6 into dev May 15, 2026
3 of 4 checks passed
@devkade devkade deleted the docs/revision-comment-rules branch May 15, 2026 09:40
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.

1 participant