refactor: extract Kapi CLI report formatters#151
Conversation
There was a problem hiding this comment.
kapi-agent review
Verdict: APPROVE
Final approval summary
Review journey
The original goal was to refactor Kapi CLI human-readable report/registry/status/list/doctor formatting out of src/cli/kapi-cli.ts into a presentation-focused module. The semantic scope is narrow: presentation formatter extraction only, with JSON payload construction and runtime/report semantics preserved. There was no prior kapi-agent feedback cycle for this PR.
What changed
- Added
src/presentation/kapi-cli-formatters.tswith extracted formatters and narrow structural view types. - Updated
src/cli/kapi-cli.tsto import and call those formatters. - Removed the moved formatter implementations from the CLI orchestration file.
Why this is correct
The extracted formatter bodies preserve the previous wording and behavior for registry entry/list/status, worker report, doctor report, GitHub issue context lines, kapi-agent PR review text, quality probe formatting, and GitHub run contract formatting. The CLI still builds and returns the same data objects through ok(..., --json, data), so JSON behavior remains unchanged.
Evidence
- Verifier gate: PASS.
- Size gate: PASS, 198 changed lines under the 200-line limit.
- Revision-explanation status: not required; found.
npm ci && npm run verifyexited 0.- Ilchul review harness gate: PASS, neutral/comment with 0 blocking findings.
- Inspected
src/cli/kapi-cli.tsandsrc/presentation/kapi-cli-formatters.ts.
Remaining risks and approval rationale
Remaining risk is low and limited to accidental human-readable output drift in future formatter changes. This PR is a bounded extraction with passing verification, no blocking findings, and no observed contract breakage, so approval is justified.
Blocking issues
None.
Warnings / risks
src/presentation/kapi-cli-formatters.ts: the new view types are intentionally structural. Future report model changes should update these views and ideally add snapshot/formatter coverage to catch CLI text drift.
Suggestions
- Consider adding focused formatter snapshot tests later for
kapi status,kapi list,kapi report, andkapi doctortext output. Not blocking for this extraction because the moved bodies are preserved and full verification passed.
Looks good
- The extraction keeps orchestration in
src/cli/kapi-cli.tswhile moving presentation concerns intosrc/presentation/kapi-cli-formatters.ts. - JSON command behavior remains tied to unchanged data construction in the CLI.
- Existing quality probe and GitHub workflow run contract presentation formatters are reused instead of reimplemented.
- The change stays within the requested narrow issue scope.
Verification notes
- Verifier gate status: PASS —
npm ci && npm run verifyexited 0. - Size gate status: PASS — 198 changed lines, below the 200-line rejection threshold.
- Revision-explanation status: not required for this first kapi-agent review; explanation found in PR body.
- Ilchul review harness: PASS, no blocking findings.
Engine: pi
0cd0aad to
1f294ab
Compare
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 kapi-agent hard limit of 200 changed lines. Please split this into multiple PRs, each under 200 changed lines and scoped to one semantic unit.
- Revision explanation gate failed: this PR was updated after a prior kapi-agent review, but there is no author explanation of what changed and why. Please add a review response or PR comment explaining the revision before requesting re-review.
Warnings / risks
- I did not perform a full semantic approval review of the current revision because the configured gates already fail.
- Since this PR was previously approved at a smaller size, the current revision needs an explicit delta explanation so reviewers can verify whether it still only performs formatter extraction and whether previous approval assumptions still hold.
Suggestions
- Split by semantic scope, for example:
- PR 1: introduce/move the presentation formatter module.
- PR 2: wire CLI imports/usages and remove old formatter bodies.
- Or another split that keeps each PR under 200 changed lines and independently reviewable.
- In the revision response, include:
- What changed since the prior kapi-agent approval.
- Why the changed-line count increased.
- Whether behavior, output wording, JSON payloads, tests, or contracts changed.
Looks good
- The stated intent remains narrow and reviewable in principle: extracting human-readable Kapi CLI formatting into
src/presentation/kapi-cli-formatters.tswhile preserving CLI JSON payload construction. - The PR body includes useful QA commands and notes, but it does not satisfy the required post-review revision explanation for the current update.
Verification notes
- Verifier gate status: FAIL.
- Size gate status: FAIL — 232 changed lines, limit is under 200.
- Revision-explanation status: FAIL — required now, not found.
- Prior kapi-agent review exists and approved an earlier revision, but the current head is not approved under the current gates.
Engine: pi
1f294ab to
6954a5b
Compare
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 review, but no author review response or PR comment explains what changed and why. Please add a clear revision response before requesting re-review, including:
- what changed since the prior kapi-agent approval / later changes-requested review,
- why the current diff is now back under the size gate,
- whether the semantic scope is still only CLI formatter extraction plus regression coverage.
Warnings / risks
- The PR body still contains a stale statement saying the current PR size is
232 changed lines, while the current measured diff is 185 changed lines. Please update that to avoid confusing reviewers and gates.
Suggestions
- In the revision explanation, explicitly call out the reduced test/formatter scope and any removed changes from the previously oversized revision.
Looks good
- Size is now within the hard gate: 185 changed lines < 200.
- Verification passed:
npm ci && npm run verifyexited 0. - Ilchul structured review harness reported no blocking findings.
Verification notes
- Verifier gate status: PASS —
npm ci && npm run verifyexited 0. - Size gate status: PASS — 185 changed lines < 200.
- Revision-explanation status: FAIL — required now, not found.
- Ilchul review harness gate: PASS / neutral comment, blocking findings: 0.
- Local diff stat confirms current scope across:
src/cli/kapi-cli.tssrc/presentation/kapi-cli-formatters.tstest/kapi-cli-formatters.test.ts
Engine: pi
|
@kapi-agent review Revision explanation for current head What changed:
Why this closes the prior feedback:
Verification:
|
1 similar comment
|
@kapi-agent review Revision explanation for current head What changed:
Why this closes the prior feedback:
Verification:
|
|
@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: APPROVE
Final approval summary
Review journey
The original goal was to extract Kapi CLI human-readable report/registry/status/list/doctor formatting from src/cli/kapi-cli.ts into a presentation module while preserving behavior. The semantic scope remains narrow: formatter relocation plus compact regression coverage only. The PR was previously approved, then blocked when a revision exceeded the 200-line gate and lacked a revision explanation. The current revision includes the required author explanation and is back under the size limit.
What changed
- Added
src/presentation/kapi-cli-formatters.tswith extracted registry, status, report, doctor, GitHub issue context, and PR review formatting. - Updated
src/cli/kapi-cli.tsto import those formatters and remove the moved implementations. - Added
test/kapi-cli-formatters.test.tscovering representative status/report/doctor wording.
Why this is correct
The extracted formatter logic preserves the prior text structure and keeps JSON data construction in src/cli/kapi-cli.ts. The new presentation types are structural views over the existing data, so this does not widen runtime, registry, GitHub adapter, quality probe, or workflow semantics.
Evidence
- Verifier gate: PASS —
npm ci && npm run verifyexited 0. - Size gate: PASS — 185 changed lines, below the 200-line hard limit.
- Revision-explanation status: PASS — author explained the current-head changes and why the size-gate feedback is closed.
- Ilchul review harness gate: PASS — neutral/comment result with 0 blocking findings.
- Inspected files:
src/cli/kapi-cli.ts,src/presentation/kapi-cli-formatters.ts,test/kapi-cli-formatters.test.ts.
Remaining risks and approval rationale
Remaining risk is low and limited to future formatter drift if CLI report shapes grow; the current change is a move/extraction with targeted regression coverage and full verification passing. Approval is justified because all gates pass, the previous blocking feedback is closed, and no blocking correctness or contract issue remains.
Blocking issues
- None.
Warnings / risks
- The new formatter view types in
src/presentation/kapi-cli-formatters.tsare intentionally narrow. Future report fields will need corresponding view/type updates to avoid presentation drift.
Suggestions
- Consider adding broader snapshot-style command behavior coverage later if these human-readable outputs become externally relied upon.
Looks good
- Clear separation of CLI orchestration from presentation formatting.
- JSON behavior remains owned by the CLI data construction path.
- Regression coverage directly exercises the extracted status/report/doctor formatters.
- Prior review feedback was addressed: current diff is under the size gate and includes a revision explanation.
Verification notes
- Verifier gate status: PASS —
npm ci && npm run verifyexited 0. - Size gate status: PASS — 185 changed lines < 200.
- Revision-explanation status: PASS — current-head explanation was provided.
- Ilchul harness: PASS — no blocking findings.
Engine: pi
|
@kapi-agent review Revision explanation for current head What changed:
Why this closes the current gate:
Verification:
|
There was a problem hiding this comment.
kapi-agent review
Verdict: APPROVE
Final approval summary
Review journey
The original goal was to extract Kapi CLI human-readable registry/status/list/report/doctor formatting from src/cli/kapi-cli.ts into a presentation module. The semantic scope remains narrow: formatter relocation plus compact regression coverage, with JSON payload construction left in the CLI layer. This PR previously ping-ponged through kapi-agent because a revision exceeded the 200-line size gate and lacked a revision explanation; the current revision includes the explanation and is back under the gate.
What changed
- Added
src/presentation/kapi-cli-formatters.tswith extracted registry, status, report, doctor, GitHub issue context, kapi-agent PR review, quality probe, and GitHub run contract formatting composition. - Updated
src/cli/kapi-cli.tsto import those formatters and remove the moved formatter bodies. - Added
test/kapi-cli-formatters.test.tscovering representative status/report/doctor wording.
Why this is correct
The moved formatter logic preserves the previous CLI text structure while keeping command orchestration and JSON response data construction in src/cli/kapi-cli.ts. The new presentation types are structural views over the existing runtime/report objects, so this does not change persistence, runtime inspection, registry semantics, quality probe behavior, or GitHub workflow contract mapping.
Evidence
- Verifier gate: PASS —
npm ci && npm run verifyexited 0. - Size gate: PASS — 185 changed lines, under the 200-line hard limit.
- Ilchul review harness gate: PASS — neutral/comment with zero blocking findings.
- Revision explanation status: found; the author explained the compaction, unchanged extraction scope, retained tests, and current head update.
- Inspected files:
src/cli/kapi-cli.ts,src/presentation/kapi-cli-formatters.ts, andtest/kapi-cli-formatters.test.ts.
Remaining risks and approval rationale
Remaining risk is low and limited to possible untested exact wording variants outside the representative formatter test cases. Approval is justified because the diff is small, gates pass, the prior size/revision-feedback issues are closed, and the implementation is a behavior-preserving extraction.
Blocking issues
- None.
Warnings / risks
test/kapi-cli-formatters.test.tscovers representative wording but not every formatter branch, such as registry list warnings, missing registry entries, GitHub issue context variants, and kapi-agent PR review variants. This is acceptable for the current extraction but worth expanding if those surfaces change again.
Suggestions
- Consider adding snapshot-style or table-driven formatter coverage in a follow-up if CLI wording becomes a stable user-facing contract.
Looks good
- The CLI module remains responsible for data construction and command flow, while presentation string assembly moves to
src/presentation/kapi-cli-formatters.ts. - The extracted formatter code preserves the key prior output ordering and fallback wording.
- The current revision closes the previous kapi-agent feedback: size is under 200 lines and the revision explanation is present.
Verification notes
- Verifier gate status: PASS —
npm ci && npm run verifyexited 0. - Size gate status: PASS — 185 changed lines < 200.
- Revision-explanation status: not required for this run and found in author comments.
- Ilchul review harness: PASS with zero blocking findings.
Engine: pi
|
@kapi-agent review Revision explanation for current head What changed:
Why this closes the current gate:
Verification:
|
ea22aba to
ee98876
Compare
There was a problem hiding this comment.
kapi-agent review
Verdict: APPROVE
Final approval summary
Review journey
The original goal was to extract Kapi CLI human-readable registry/status/list/report/doctor formatting out of src/cli/kapi-cli.ts into a presentation module. The semantic scope remains narrow: formatter relocation plus compact regression coverage, while JSON payload construction stays in the CLI layer. This PR previously ping-ponged through kapi-agent because one revision exceeded the 200-line gate and lacked a revision explanation; the current revision includes the explanation and is under the gate.
What changed
- Added
src/presentation/kapi-cli-formatters.tswith extracted formatters and structural presentation view types. - Updated
src/cli/kapi-cli.tsto import those formatters and remove the moved implementations. - Added
test/kapi-cli-formatters.test.tscovering representative status/report/doctor text output.
Why this is correct
The extracted formatter logic preserves the prior output structure for registry status/list/entry, worker report, doctor report, GitHub issue context, kapi-agent PR review, quality probe, and GitHub run contract sections. Runtime orchestration and ok(..., --json, data) data construction remain in src/cli/kapi-cli.ts, so the change is presentation-only rather than behavior-changing.
Evidence
- Verifier gate: PASS —
npm ci && npm run verifyexited 0. - Size gate: PASS — 185 changed lines, below the 200-line hard limit.
- Revision-explanation status: not required for this synchronize event, and an author explanation is present.
- Ilchul harness gate: PASS — neutral/comment, no blocking findings.
- Inspected files:
src/presentation/kapi-cli-formatters.ts,test/kapi-cli-formatters.test.ts, and the summarized diff forsrc/cli/kapi-cli.ts.
Remaining risks and approval rationale
Remaining risk is low and limited to human-readable wording drift in compacted formatter code. The representative formatter tests plus full verify run are sufficient for this small presentation extraction, and all configured gates pass, so approval is justified.
Blocking issues
- None.
Warnings / risks
- The direct formatter test is representative rather than exhaustive for every report branch, so future formatter changes should add focused cases for any newly formatted field or conditional section.
Suggestions
- Consider adding snapshot-like coverage for
formatRegistryListandformatRegistryEntryin a follow-up if these outputs become part of a stricter CLI contract.
Looks good
- The CLI orchestration file now delegates report/registry/doctor presentation concerns to
src/presentation/kapi-cli-formatters.ts. - JSON behavior remains protected by keeping payload construction in
src/cli/kapi-cli.ts. - The revision explanation closes the earlier review feedback about the oversized intermediate revision and confirms the scope did not widen.
Verification notes
- Verifier gate status: PASS —
npm ci && npm run verifyexited 0. - Size gate status: PASS — 185 changed lines < 200.
- Revision-explanation status: found; not required now.
- Ilchul structured review harness: PASS with no blocking findings.
Engine: pi
Summary
src/cli/kapi-cli.tsintosrc/presentation/kapi-cli-formatters.ts.Linked issue
Closes #134
Problem
src/cli/kapi-cli.tsmixed command orchestration, runtime inspection, registry/doctor/report modeling, and CLI text presentation in one large module. Issue #134 asks for a narrow CLI/presentation extraction while preserving currentkapi report,kapi status,kapi list, andkapi doctorbehavior.Options considered
kapi-cli.tsfor possible later cleanup.Selected approach
Option 2: a narrow extraction. This keeps the change small and reviewable while directly addressing the issue scope. Events/help/probe/start formatting remain in
kapi-cli.tsbecause they are outside the requested report/registry/doctor formatter extraction.Implementation by file/surface
src/presentation/kapi-cli-formatters.tssrc/cli/kapi-cli.tsok(..., --json, data).test/kapi-cli-formatters.test.tsWhy this fixes it
The CLI orchestration module no longer owns report/registry/doctor human-readable formatter bodies. Formatting now lives in a narrow presentation module, while JSON behavior remains driven by unchanged report/registry/doctor data construction in
kapi-cli.ts.QA / Verification
RED evidence:
/private/tmp/ilchul-issue-134/node_modules/.bin/tsx --test test/presentation-command-behavior.test.tsin anorigin/devworktree with the new formatter test copied in — failed withERR_MODULE_NOT_FOUNDforsrc/presentation/kapi-cli-formatters.js, proving the extracted formatter module was missing on the base branch.GREEN / regression:
./node_modules/.bin/tsx --test test/kapi-cli-formatters.test.ts— pass, 1 test.npm test -- test/cli-args.test.ts test/presentation-command-behavior.test.ts test/kapi-cli-formatters.test.ts— pass, 393 passed / 11 skipped. Note: the package script expandstest/*.test.ts, so this ran the full test glob plus the requested files.npm run check— pass.npm run check:unused— pass.npm run quality:budgets— pass with existing budget warning:code_smells=50vs<=20; command exit code 0.git diff --check— pass.Anomalies observed
tsx --test test/*.test.ts; reported results reflect the actual broad run.DEP0205 module.register()deprecation warnings.src/cli/kapi-review-cli.tsto executable mode locally; unrelated mode churn was reverted before push.Risks / Follow-up
kapi-agent review expectations and current-head merge gate
kapi status,kapi list,kapi report, andkapi doctortext output.--jsonpayloads remain unchanged because data construction stayed inkapi-cli.ts.106 insertions + 79 deletions) againstorigin/dev, under the kapi-agent 200 changed-line hard gate.kapi-agentapproval andkapi-agent/reviewsuccess are present.