Skip to content

refactor: rename presentation workflow helpers#213

Merged
devkade merged 1 commit into
devfrom
refactor/issue-209-presentation-helper-names
May 17, 2026
Merged

refactor: rename presentation workflow helpers#213
devkade merged 1 commit into
devfrom
refactor/issue-209-presentation-helper-names

Conversation

@devkade
Copy link
Copy Markdown
Owner

@devkade devkade commented May 17, 2026

Summary

  • Renames generic presentation helper/type exports away from product-prefixed Kapi* names.
  • Updates internal imports and hook-policy tests to use semantic workflow/tool helper names.
  • Adds an architecture guard preventing the old generic presentation helper names from returning.

Linked issue

Refs #209

Problem

Issue #209 is removing historical kapi product-name leakage from reusable internal code while preserving intentional public compatibility surfaces.

After the service filename/factory slices, several generic presentation helpers still used product-prefixed internal names:

KapiToolDefinition
shouldBlockKapiToolCall
formatKapiError

These helpers are not serialized workflow IDs, public command labels, or kapi-agent integration contracts. They are generic presentation/tool plumbing and should use semantic names.

Options considered

  1. Rename only generic helper/type exports.
    • Pros: small, low-risk, keeps public labels/tool names untouched.
    • Cons: larger service/store/registry symbols remain for later slices.
  2. Rename all presentation Kapi text at once.
    • Pros: larger visible cleanup.
    • Cons: would mix internal helper names with public labels, status keys, and tool names that are compatibility/user-facing surfaces.
  3. Skip presentation helper names until service/store symbols are renamed.
    • Pros: avoids another small PR.
    • Cons: leaves easy internal leakage in code that is already isolated and safe to rename.

Selected approach

Selected option 1.

This PR renames only generic internal presentation helper/type exports. It deliberately leaves public Kapi labels, kapi_start_workflow tool names, kapi-workflow event labels, /kapi-* workflow IDs, kapi-agent, and kapi-review untouched.

Implementation by file/surface

  • src/presentation/tools.ts
    • KapiToolDefinitionWorkflowToolDefinition.
    • Uses formatWorkflowError for tool error formatting.
  • src/presentation/hooks.ts
    • shouldBlockKapiToolCallshouldBlockWorkflowToolCall.
  • src/presentation/ui.ts
    • formatKapiErrorformatWorkflowError.
  • src/presentation/tool-ui-actions.ts and src/presentation/command-ui-actions.ts
    • Re-export/import the semantic error formatter name.
  • test/hook-policy.test.ts
    • Updates hook-policy imports and assertions to the semantic hook helper name.
  • test/architecture.test.ts
    • Adds a guard for the old generic presentation helper names and asserts the new semantic names exist.
  • docs/product-name-audit.md
    • Records this bounded presentation-helper cleanup.

Why this fixes it

The generic presentation helper exports no longer carry product-prefixed names. The architecture test now fails if KapiToolDefinition, shouldBlockKapiToolCall, or formatKapiError re-enter presentation source.

This is still a partial #209 slice, so this PR uses Refs #209 rather than Closes #209.

QA / Verification

  • npm test -- test/architecture.test.ts test/hook-policy.test.ts test/presentation-command-behavior.test.ts — pass; package script ran the full suite, 521 tests, 510 pass, 11 skipped.
  • 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.

Residual old helper-name scan:

rg -n 'KapiToolDefinition|shouldBlockKapiToolCall|formatKapiError' src test docs
# only the architecture negative assertion remains

Changed-line count:

git diff --shortstat origin/dev...HEAD
8 files changed, 25 insertions(+), 16 deletions(-)

Independent review:

  • Static/security scan — no hardcoded secrets, shell injection keywords, eval/exec, unsafe deserialization, or SQL injection findings.
  • Independent reviewer subagent — passed; no blocking logic/security issues.

Anomalies observed

Symptom Evidence Impact Action
Targeted-looking npm test -- <files> runs the full suite because the package script is tsx --test test/*.test.ts. Test output showed 521 tests. QA was broader than the named files; runtime cost only. Reported exact scope above.
src/cli/kapi-review-cli.ts executable-bit churn recurred during verification. Mode-only churn appeared after verification. Could add unrelated mode churn. Reset mode before staging; no mode-only churn committed.

Risks / Follow-up

  • Larger exported names such as KapiService, KapiStore, KapiRegistryEntry, and KapiAgentPrReviewState remain for later Remove remaining kapi identifiers from internal codebase #209 slices.
  • Public labels/tool names still intentionally use Kapi where they are compatibility or user-facing surfaces.
  • kapi-review, kapi-agent, /kapi-*, and serialized kapi-* workflow IDs remain intentional compatibility/external integration surfaces.

kapi-agent review

  • Expected reviewer-only bot behavior: automatic formal review after PR open; if absent after a short wait, request with @kapi-agent review.
  • Merge gate: latest kapi-agent approval/check must apply to the current head before merge.
  • Re-review rule: after stale approval, REQUEST_CHANGES, or COMMENT, request re-review with a same-comment current-head revision explanation.

@devkade
Copy link
Copy Markdown
Owner Author

devkade commented May 17, 2026

@kapi-agent review

Revision explanation for current head 62629d20488d8270f7f9dfdb9f1f4577a6fbeb74:

What changed:

  • Renamed generic presentation helper/type exports from KapiToolDefinition, shouldBlockKapiToolCall, and formatKapiError to semantic workflow/tool names.
  • Updated internal imports, hook-policy tests, and product-name audit docs.
  • Added an architecture guard preventing those old generic helper names from returning.

Why this closes the prior feedback:

  • This is a bounded partial Remove remaining kapi identifiers from internal codebase #209 slice focused only on generic presentation helper exports.
  • Public labels/tool names/status keys such as Kapi, kapi_start_workflow, kapi-workflow, /kapi-*, kapi-agent, and kapi-review remain intentionally unchanged as compatibility or external integration surfaces.
  • The only old helper-name occurrence is the architecture negative assertion.

Verification:

  • npm test -- test/architecture.test.ts test/hook-policy.test.ts test/presentation-command-behavior.test.ts — pass; package script ran the full suite, 521 tests, 510 pass, 11 skipped.
  • 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.
  • Independent reviewer subagent — 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: APPROVE

Final approval summary

Review journey

This PR is a bounded refactor slice for issue #209. The semantic scope is limited to renaming generic internal presentation workflow/tool helper exports away from product-prefixed Kapi* names while preserving public compatibility surfaces such as tool names, labels, workflow IDs, and the kapi-agent integration. There is no prior kapi-agent review ping-pong on this PR.

What changed

The current revision updates:

  • src/presentation/tools.ts: KapiToolDefinitionWorkflowToolDefinition
  • src/presentation/hooks.ts: shouldBlockKapiToolCallshouldBlockWorkflowToolCall
  • src/presentation/ui.ts: formatKapiErrorformatWorkflowError
  • Presentation imports/re-exports in command-ui-actions.ts and tool-ui-actions.ts
  • Tests in test/hook-policy.test.ts and test/architecture.test.ts
  • Audit documentation in docs/product-name-audit.md

Why this is correct

The change keeps runtime behavior intact: user-facing strings and external contracts still intentionally use Kapi names, while only internal generic helper identifiers are renamed. The new architecture guard prevents the old generic presentation helper names from re-entering src/presentation.

Evidence

Verifier gate: PASS. npm ci && npm run verify completed successfully.
Size gate: PASS. 41 changed lines, below the 200-line semantic review threshold.
Revision-explanation status: not required for this initial kapi-agent review, and explanation is present in the PR body.
Ilchul review harness gate: PASS with neutral/comment output and zero blocking findings.
I inspected the changed source, test, and audit paths and confirmed the rename is consistent.

Remaining risks and approval rationale

Remaining risk is low and limited to internal import churn. Larger product-prefixed exported names such as KapiService and related types remain intentionally out of scope for later slices. Approval is justified because the scope is narrow, tests and architecture guards pass, and no blocking correctness, safety, or workflow-contract issue remains.

Blocking issues

None.

Warnings / risks

  • src/presentation/ui.ts still returns the user-facing message prefix Kapi action failed:. This appears intentional because public/user-facing Kapi surfaces are explicitly preserved.
  • Broader product-name cleanup remains incomplete by design; future slices should continue to distinguish internal reusable identifiers from persisted/public contracts.

Suggestions

  • In a later slice, consider adding similarly targeted architecture guards for the next planned internal identifiers once their compatibility boundary is decided.

Looks good

  • The refactor is mechanically consistent across imports, exports, and call sites.
  • The architecture test directly protects against regression of the renamed presentation helpers.
  • Public tool names, event labels, workflow IDs, and integration names remain untouched, preserving artifact/workflow contract compatibility.

Verification notes

Verifier gate: PASS — npm ci && npm run verify exited 0.
Size gate: PASS — 41 changed lines, with low semantic risk.
Revision-explanation status: not required; found.
Ilchul harness: PASS — neutral/comment, zero blocking findings.


Engine: pi

@devkade devkade merged commit 6d53328 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