refactor: consolidate Track A RunContract formatting#132
Merged
Conversation
![kapi-agent[bot]](https://avatars.githubusercontent.com/in/3661577?s=60&v=4){kind=small}
There was a problem hiding this comment.
kapi-agent review
Verdict: REQUEST_CHANGES
Blocking issues
- Size gate failed: this PR has 232 changed lines, which exceeds the 200-line hard limit. Please split this into multiple PRs, each under 200 changed lines and scoped to one semantic unit.
- Because the verifier gate failed, I cannot approve this revision even if the refactor appears mechanically reasonable.
Suggested split:
- PR 1: extract
src/presentation/run-contract-format.tsfromsrc/presentation/messages.tswith focused tests. - PR 2: refactor
src/domain/run-contract-prompt-renderer.tshelper formatting and its prompt-renderer tests.
Warnings / risks
- The touched area is artifact/workflow-contract presentation logic, where small formatting changes can affect downstream prompt/workflow expectations. Please keep each split PR narrow enough that wording-preservation can be reviewed independently.
Suggestions
- In the follow-up PR descriptions, explicitly state which output strings are intended to remain byte-for-byte stable and which are intentionally reformatted.
- Include targeted test commands for each split PR, not only the full verifier.
Looks good
- The extracted
run-contract-format.tsappears to preserve the prior compact RunContract summary behavior. - Added direct tests for RunContract formatter behavior are the right kind of regression coverage for this refactor.
Verification notes
- Verifier gate status: FAIL — size gate failed: 232 changed lines >= 200.
- Size gate status: FAIL — must be below 200 changed lines.
- Revision-explanation status: not required for this opened PR; found true.
- Reported author verification:
npm test -- test/run-contract-format.test.ts test/run-contract-prompt-renderer.test.ts test/presentation-ui.test.ts,npm run verify, andgit diff --check, but approval is blocked by the size gate.
Engine: pi
This was referenced May 15, 2026
Owner
Author
|
@kapi-agent review Revision explanation for current head What changed:
Why this closes the prior feedback:
Verification:
|
Owner
Author
|
@kapi-agent review Revision explanation for current head What changed:
Why this closes the prior feedback:
Verification:
|
There was a problem hiding this comment.
kapi-agent review
Verdict: REQUEST_CHANGES
Blocking issues
-
Revision explanation gate failed. This PR was updated after a prior kapi-agent changes-requested review, but there is no author comment or PR-body update explaining what changed and why. Please add a review response summarizing:
- what changed since the previous kapi-agent review,
- why those changes address the prior size/verifier feedback,
- any remaining intentional scope.
After that, request re-review.
Warnings / risks
- The touched code is RunContract/workflow prompt presentation logic (
src/domain/run-contract-prompt-renderer.ts,src/presentation/messages.ts,src/presentation/run-contract-format.ts). Even formatting-only refactors can affect downstream prompt expectations, so preserving exact wording and section spacing remains important. formatKapiRules()now builds the same runtime guidance through line arrays. The current tests appear to cover this, but any future edits should keep byte-for-byte prompt output regression tests close to this code.
Suggestions
- Add the missing revision response as a PR comment rather than relying only on commit messages, so the review history clearly shows how the prior feedback was addressed.
- In the revision response, explicitly call out that the PR is now under the 200-line limit and that
npm run verifypasses.
Looks good
- Size is now within the hard gate: 190 changed lines.
- The extraction from
src/presentation/messages.tstosrc/presentation/run-contract-format.tsappears mechanically scoped and preserves the existing compact RunContract summary wording. - The helper extraction in
src/domain/run-contract-prompt-renderer.tsimproves maintainability without obvious adapter/runtime coupling. - Added regression coverage in
test/run-contract-format.test.tsdirectly protects the newly extracted presentation formatter behavior.
Verification notes
- Verifier gate status: FAIL, because the revision explanation gate failed.
- Size gate status: PASS — 190 changed lines, under the 200-line limit.
- Revision-explanation status: FAIL — prior kapi-agent review exists, this is a revised PR, and no explanation of what changed and why was found.
- CI/local evidence provided:
npm ci && npm run verifyexited 0, including test, check, unused check, and quality budget commands.
Engine: pi
Owner
Author
|
@kapi-agent review Revision explanation for current head What changed:
Why this closes the prior feedback:
Verification:
|
kapi-agent reviewVerdict: REQUEST_CHANGES Blocking issues
Warnings / risks
Suggestions
Looks good
Verification notes
Engine: kapi-agent request lifecycle:
|
There was a problem hiding this comment.
kapi-agent review
Verdict: REQUEST_CHANGES
Blocking issues
- Verification gate failed: the Ilchul review harness gate failed with a merge-blocking finding:
Ilchul review CLI failed- Evidence:
Unexpected kapi-review arguments: --input-file .../review-input.json
- Per gate policy, I cannot approve while any configured gate is FAIL, even though the size gate and normal verifier passed.
Warnings / risks
- The changed code is workflow-contract and RunContract presentation logic:
src/domain/run-contract-prompt-renderer.tssrc/presentation/messages.tssrc/presentation/run-contract-format.ts
- Formatting refactors in this area can create regressions in prompt/workflow contract expectations even when behavior looks mechanically equivalent. The new focused tests reduce this risk, but byte-for-byte prompt stability remains important.
Suggestions
- Fix the Ilchul review harness invocation/configuration so it accepts the current review input contract or update the caller to use the expected CLI arguments.
- After the harness is fixed, request re-review without expanding this PR’s semantic scope.
- Consider adding a small regression test around the Ilchul/kapi-review CLI argument contract if this failure came from project code rather than external review infrastructure.
Looks good
- Size gate now passes: 190 changed lines, under the 200-line hard limit.
- The revision explanation is present and directly addresses the prior kapi-agent feedback.
- The extraction from
src/presentation/messages.tstosrc/presentation/run-contract-format.tsappears mechanically scoped and preserves the existing compact RunContract wording. - The prompt-renderer helper refactor keeps the same section ordering and readable construction while improving maintainability.
- Added tests cover the new RunContract formatter surface and unknown workflow behavior.
Verification notes
- Verifier gate status: FAIL due to Ilchul review harness failure.
- Size gate status: PASS, 190 changed lines < 200.
- Revision-explanation status: PASS, explanation found.
- Standard verifier evidence: PASS,
npm ci && npm run verifyexited 0. - Reviewed evidence includes the changed files and tests listed in the PR context, especially:
src/domain/run-contract-prompt-renderer.tssrc/presentation/messages.tssrc/presentation/run-contract-format.tstest/run-contract-format.test.ts
Engine: pi
Owner
Author
|
@kapi-agent review Revision explanation for current head What changed:
Why this closes the prior feedback:
Verification:
|
There was a problem hiding this comment.
kapi-agent review
Verdict: REQUEST_CHANGES
Blocking issues
- Revision explanation gate failed. This PR was updated after prior kapi-agent reviews, but no revision explanation was found for the current head
d4e6d4b4b6eb5530f3e503f0a031d40826096fd0. The existing author comments explain earlierfae23f...heads, not the current revision.- Please add a PR comment or PR body update explaining:
- what changed since the last kapi-agent review,
- why the current head closes the prior feedback,
- whether the semantic scope changed,
- what verification was run for this exact revision.
- After that, request re-review.
- Please add a PR comment or PR body update explaining:
Warnings / risks
- The changed area remains workflow-contract and RunContract presentation logic:
src/domain/run-contract-prompt-renderer.tssrc/presentation/messages.tssrc/presentation/run-contract-format.ts
- Formatting-only refactors in this area can still affect downstream prompt/workflow expectations. The added regression tests reduce that risk, but byte-for-byte wording and spacing should remain explicitly protected.
Suggestions
- In the revision response, explicitly call out whether the current
d4e6d4...revision only preserves the prior 190-line scoped refactor or includes any additional changes. - Consider keeping future prompt-formatting changes in similarly focused PRs with direct output regression tests.
Looks good
- Size gate now passes: 190 changed lines is under the 200-line hard limit.
- Standard verification passed:
npm ci && npm run verifyexited 0. - The extracted
src/presentation/run-contract-format.tsappears mechanically equivalent to the previous inline formatter inmessages.ts. - The new formatter tests cover compact RunContract summary wording and unknown workflow handling.
- The prompt-renderer refactor is organized into smaller helpers without obvious adapter/runtime boundary leakage.
Verification notes
- Verifier gate status: FAIL overall, because the revision explanation gate failed.
- Size gate status: PASS, 190 changed lines < 200.
- Revision-explanation status: FAIL for current head
d4e6d4b4b6eb5530f3e503f0a031d40826096fd0. - Normal verifier status: PASS,
npm ci && npm run verifyexited 0. - Ilchul review harness status: PASS/neutral, no blocking findings.
Engine: pi
Owner
Author
|
@kapi-agent review |
kapi-agent reviewVerdict: APPROVE Final approval summaryReview journeyThe original goal was to consolidate Track A RunContract formatting while preserving workflow/prompt behavior. The semantic scope is limited to extracting compact RunContract presentation helpers from What changed
Why this is correctThe extraction preserves the same compact RunContract wording and keeps presentation-specific formatting outside the larger messages module. The prompt renderer refactor is mechanical: it builds the same sections through helper functions while preserving workflow/runtime layout boundaries and avoiding adapter-specific semantics. Evidence
Remaining risks and approval rationaleRemaining risk is low and limited to prompt wording regressions in workflow contract output. The focused regression tests cover the extracted formatter and prompt-renderer boundaries, all configured gates pass, and no blocking issue remains, so approval is justified. Blocking issues
Warnings / risks
Suggestions
Looks good
Verification notes
Engine: kapi-agent request lifecycle:
|
There was a problem hiding this comment.
kapi-agent review
Verdict: APPROVE
Final approval summary
Review journey
The original goal was to consolidate Track A RunContract formatting while preserving workflow/prompt behavior. The semantic scope is limited to extracting compact RunContract presentation helpers from src/presentation/messages.ts and simplifying renderRunContractPrompt formatting helpers in src/domain/run-contract-prompt-renderer.ts. Prior kapi-agent feedback blocked on PR size, then missing/current revision explanation, then a review harness contract failure; the current revision includes an author explanation for the current head and aligns with origin/dev to restore the harness.
What changed
- Added
src/presentation/run-contract-format.tsfor RunContract presentation formatting. - Updated
src/presentation/messages.tsto delegate to the extracted formatter. - Refactored
src/domain/run-contract-prompt-renderer.tsinto small section-formatting helpers. - Added/updated tests in:
test/run-contract-format.test.tstest/run-contract-prompt-renderer.test.ts
Why this is correct
The extraction preserves the same compact RunContract wording and keeps presentation-specific formatting outside the larger messages module. The prompt renderer refactor is mechanical: it builds the same sections through helper functions while preserving workflow/runtime layout boundaries and avoiding adapter-specific semantics.
Evidence
- Verifier gate: PASS.
- Size gate: PASS, 190 changed lines under the 200-line hard limit.
- Revision-explanation status: PASS, explanation found for current head
d4e6d4b4b6eb5530f3e503f0a031d40826096fd0. - Ilchul review harness gate: PASS, neutral/comment result with 0 blocking findings.
npm ci && npm run verifyexited 0.- Local inspection included
git diff --stat origin/dev...HEAD,git diff --check origin/dev...HEAD, and the changed RunContract prompt/formatter tests.
Remaining risks and approval rationale
Remaining risk is low and limited to prompt wording regressions in workflow contract output. The focused regression tests cover the extracted formatter and prompt-renderer boundaries, all configured gates pass, and no blocking issue remains, so approval is justified.
Blocking issues
- none.
Warnings / risks
- Prompt/workflow contract formatting remains a sensitive area. Future edits to
src/domain/run-contract-prompt-renderer.tsorsrc/presentation/run-contract-format.tsshould keep byte-for-byte wording tests close to the changed text.
Suggestions
- Consider snapshot-style or exact-string tests for full workflow/runtime prompt outputs if this formatting continues to evolve.
Looks good
- The extracted
run-contract-format.tskeeps RunContract UI formatting cohesive and testable. messages.tsis simpler after delegating RunContract-specific formatting.- The helper split in
run-contract-prompt-renderer.tsimproves maintainability without adding adapter-specific concepts. - Tests cover the compact formatter output, unknown workflow behavior, shared renderer path, and runtime adapter-neutrality.
Verification notes
- Verifier gate: PASS —
npm ci && npm run verifyexited 0. - Size gate: PASS — 190 changed lines, below the 200-line limit.
- Revision-explanation status: PASS — current-head explanation found.
- Ilchul review harness gate: PASS — no blocking findings.
- Local evidence:
git diff --check origin/dev...HEADpassed.
Engine: pi
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
1 participant
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
src/presentation/messages.tsintosrc/presentation/run-contract-format.ts.renderRunContractPromptinto small formatting helpers while preserving prompt wording and adapter-neutral semantics.Refactor target evidence
src/presentation/messages.ts: inlineformatRunContractSection,formatRunContractInline,missingExpectationCount, andrunContractForStatewere presentation helpers embedded in the larger UI formatter module.src/domain/run-contract-prompt-renderer.ts: long template-literal sections for runtime context, artifact cadence, shared lifecycle, and Kapi rules were drift-prone after refactor: unify RunContract prompt rendering across service and CLI runtime #115's shared renderer consolidation.test/run-contract-prompt-renderer.test.ts: setup duplicated long option objects; split fixtures make the shared renderer contract easier to extend without hiding wording changes.Boundaries preserved
workflow-validationremains the completion authority; formatter changes only project and display the existing advisory RunContract view.Verification
npm test -- test/run-contract-format.test.ts test/run-contract-prompt-renderer.test.ts test/presentation-ui.test.tsnpm run verifygit diff --checkCloses #133
Part of #128
Revision explanation for current head
d4e6d4b4b6eb5530f3e503f0a031d40826096fd0What changed:
origin/dev, bringing in the current review harness CLI contract from PR feat: add review runner command hook #147.Why this closes the prior feedback:
fae23f...identified a review harness invocation failure caused by the branch being behinddev.d4e6d4b4b6eb5530f3e503f0a031d40826096fd0includes currentdev, so that harness contract is present.Verification for this head:
git diff --check origin/dev...HEADpassed.npm run verifypassed after alignment.