Skip to content

refactor: rename review gate helper exports#224

Merged
devkade merged 1 commit into
devfrom
refactor/issue-209-review-gate-exports
May 17, 2026
Merged

refactor: rename review gate helper exports#224
devkade merged 1 commit into
devfrom
refactor/issue-209-review-gate-exports

Conversation

@devkade
Copy link
Copy Markdown
Owner

@devkade devkade commented May 17, 2026

Summary

  • rename kapi-review helper exports from product-prefixed names to semantic review-gate names
  • update tests to import/use the semantic helper names while preserving CLI behavior
  • add an architecture guard and audit note for the old exported helper names

Linked issue

Refs #209

Problem

Issue #209 is removing reusable product-name leakage from active code while preserving public and persisted contracts. The kapi-review CLI is an external integration and should remain literal, but its exported TypeScript helper names (KapiReviewInput, KapiReviewResult, buildKapiReview, runKapiReviewCli, parseKapiReviewArgs, KAPI_REVIEW_MAX_CHANGED_LINES) were reusable implementation identifiers.

Options considered

  1. Leave review CLI helpers unchanged because the CLI itself is kapi-review.
    • Pros: zero churn.
    • Cons: leaves reusable exported code identifiers product-prefixed.
  2. Rename helper exports only, preserving CLI filename, usage text, output, and kapi-agent contract text.
    • Pros: removes reusable-code leakage without touching external integration contracts.
    • Cons: updates test imports/call sites.
  3. Rename the kapi-review CLI/file/integration too.
    • Pros: broader literal cleanup.
    • Cons: unsafe and out of scope because the GitHub review integration still uses those external names.

Selected approach

Selected option 2. This is intentionally a helper/API rename only:

  • KapiReviewInputReviewGateInput
  • KapiReviewResultReviewGateResult
  • buildKapiReviewbuildReviewGate
  • runKapiReviewClirunReviewGateCli
  • parseKapiReviewArgsparseReviewGateArgs
  • KAPI_REVIEW_MAX_CHANGED_LINESREVIEW_GATE_MAX_CHANGED_LINES

Implementation by file/surface

  • src/cli/kapi-review-cli.ts: semantic helper/export names; CLI strings remain kapi-review and kapi-agent where externally visible.
  • test/kapi-review-cli.test.ts: imports and call sites use semantic helper names.
  • test/architecture.test.ts: guards against the old product-prefixed helper exports returning.
  • docs/product-name-audit.md: records this cleanup slice.

Why this fixes it

The public external review integration remains stable, while reusable TypeScript identifiers now describe the generic review gate they implement instead of carrying product branding.

QA / Verification

  • npm test -- test/kapi-review-cli.test.ts test/architecture.test.ts — pass; package script runs the full test/*.test.ts suite plus the requested files.
  • npm run check — pass.
  • npm run check:unused — pass.
  • npm run quality:budgets — pass with existing non-failing code_smells=52 warning.
  • git diff --check — pass.

Anomalies observed

  • Package npm test expands test/*.test.ts, so targeted invocations still execute the full suite.

Risks / Follow-up

  • Risk is low: rename-only helper surface with passing tests and unchanged CLI/user-facing strings.
  • Follow-up remains: continue classifying test fixture/local-variable product names versus intentional public/persisted Kapi contract text.

kapi-agent review expectations

  • Current-head kapi-agent approval and kapi-agent/review success are required before merge.

@devkade devkade force-pushed the refactor/issue-209-review-gate-exports branch from 49936d5 to 3124927 Compare May 17, 2026 12:37
@devkade
Copy link
Copy Markdown
Owner Author

devkade commented May 17, 2026

@kapi-agent review

Head SHA: 3124927

What changed:

  • Renamed product-prefixed kapi-review helper exports to semantic review-gate names.
  • Preserved the external kapi-review CLI, usage/output strings, and kapi-agent contract text.
  • Added an architecture guard and audit-doc entry for the old helper names.

Why this closes prior feedback:

Verification:

  • npm test -- test/kapi-review-cli.test.ts test/architecture.test.ts (package script ran full suite plus requested files)
  • npm run check
  • npm run check:unused
  • npm run quality:budgets
  • git diff --check

Residual scan:

rg -n 'KapiReviewInput|KapiReviewResult|buildKapiReview|runKapiReviewCli|parseKapiReviewArgs|KAPI_REVIEW_MAX_CHANGED_LINES' src/cli/kapi-review-cli.ts test/kapi-review-cli.test.ts docs/product-name-audit.md
# no matches outside the architecture regression guard

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: remove product-prefixed reusable TypeScript helper names from the kapi-review review gate implementation while preserving external kapi-review CLI and kapi-agent contract strings.
  • The semantic scope is narrow: helper/interface/function/constant naming, tests, one architecture guard, and the product-name audit note.
  • No prior kapi-agent blocking feedback exists for this PR, so no ping-pong fixes were required.

What changed

  • src/cli/kapi-review-cli.ts now uses semantic review-gate names: ReviewGateInput, ReviewGateResult, buildReviewGate, runReviewGateCli, parseReviewGateArgs, and REVIEW_GATE_MAX_CHANGED_LINES.
  • test/kapi-review-cli.test.ts imports and exercises the new helper names.
  • test/architecture.test.ts guards against reintroducing the old product-prefixed helper names.
  • docs/product-name-audit.md records this cleanup slice.

Why this is correct

  • The user-facing and integration-facing strings remain stable where they matter: the CLI usage still says kapi-review, the bin name is unchanged, and the review body/check contract remains kapi-agent.
  • The rename does not alter gate logic, artifact persistence, runner validation, risk classification, or CLI argument semantics.
  • The architecture guard directly protects the intended invariant.

Evidence

  • Verifier gate: PASS.
  • Size gate: PASS, 103 changed lines under the 200-line semantic threshold.
  • Revision-explanation status: not required; found.
  • Ilchul review harness gate: PASS, neutral/comment, no blocking findings.
  • CI/local evidence provided: npm ci && npm run verify exited 0, including tests, typecheck, unused checks, and quality budgets.
  • Inspected files: src/cli/kapi-review-cli.ts, test/kapi-review-cli.test.ts, test/architecture.test.ts, docs/product-name-audit.md, and package.json.

Remaining risks and approval rationale

  • Remaining risk is low: this is a private package and current internal call sites/tests are migrated.
  • The kapi-review CLI file mode changed to executable, which is acceptable for a bin target with a shebang.
  • Approval is justified because all gates pass, no blocking correctness/security/workflow-contract issue remains, and tests cover the renamed helper surface.

Blocking issues

  • none.

Warnings / risks

  • src/cli/kapi-review-cli.ts: the file mode changed from 100644 to 100755. This is likely fine for a CLI bin with a shebang, but it is worth keeping intentional in the final diff.

Suggestions

  • Consider exporting REVIEW_GATE_MAX_CHANGED_LINES only if it is meant to be a supported helper API. The PR body calls it an exported helper name, but the implementation keeps it as an internal constant.

Looks good

  • External CLI and contract text are preserved while internal reusable identifiers become semantic.
  • Tests were updated at the import/call-site level rather than relying only on a text scan.
  • The new architecture guard makes the naming cleanup regression-resistant.
  • Documentation was updated in the product-name audit, keeping the implementation and audit trail aligned.

Verification notes

  • Verifier gate status: PASS — npm ci && npm run verify exited 0.
  • Size gate status: PASS — 103 changed lines, below threshold.
  • Revision-explanation status: not required for this review; found.
  • Ilchul harness: PASS with no blocking findings.

Engine: pi

@devkade devkade merged commit 811120b into dev May 17, 2026
2 checks passed
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