Skip to content

test: migrate workflow service imports#220

Merged
devkade merged 1 commit into
devfrom
refactor/issue-209-workflow-service-tests-1
May 17, 2026
Merged

test: migrate workflow service imports#220
devkade merged 1 commit into
devfrom
refactor/issue-209-workflow-service-tests-1

Conversation

@devkade
Copy link
Copy Markdown
Owner

@devkade devkade commented May 17, 2026

Summary

  • Migrates the first batch of small service tests from the temporary KapiService compatibility export to the semantic WorkflowService import/class name.
  • Updates docs/product-name-audit.md so the audit now identifies only the large service-store and autoresearch-validation test clusters as remaining compatibility-import work.
  • Keeps this slice reviewable at 176 changed lines against dev and leaves the compatibility alias in place for the remaining clusters.

Linked issue

Refs #209

Problem

Issue #209 is removing reusable internal kapi product-name identifiers while preserving intentional external or serialized compatibility surfaces. PR #219 renamed the source-facing service class to WorkflowService, but many tests still imported and constructed the temporary KapiService alias.

For this slice, the problematic surface was test-only import/use leakage:

small service tests should import and instantiate WorkflowService directly;
large remaining compatibility clusters stay isolated for follow-up slices.

Options considered

  1. Migrate every remaining test in one PR — update service-store, autoresearch-validation, and all small tests at once.
    • Pros: removes most alias consumers quickly.
    • Cons: likely exceeds the review-size gate and mixes large clusters with simple imports.
  2. Migrate only small test files first — update low-risk test imports/constructors and leave the two large clusters for dedicated slices.
    • Pros: reviewable, mechanical, keeps alias removal safe.
    • Cons: leaves KapiService compatibility import debt temporarily.
  3. Do nothing until alias removal — keep tests on the alias until a final cleanup PR.
    • Pros: no immediate churn.
    • Cons: hides remaining work and keeps product-name leakage spread across many files.

Selected approach

Selected option 2.

This PR migrates the low-risk small test files first, keeps the temporary alias for the two large clusters, and updates the audit so the remaining work is explicit. It deliberately avoids broad source changes or alias removal in this slice.

Implementation

  • test/*
    • Migrated 27 small test files from KapiService to WorkflowService imports and constructors/type references.
    • Left test/service-store.test.ts and test/autoresearch-validation.test.ts unchanged for dedicated follow-up slices.
  • docs/product-name-audit.md
    • Documents that the first small-test batch now uses WorkflowService.
    • Narrows remaining KapiService compatibility-import work to the two large clusters.

Why this fixes it

The small test files no longer depend on the temporary KapiService compatibility export, so the remaining service alias consumers are concentrated and auditable. This reduces internal product-name leakage without touching serialized /kapi-* workflow IDs, public compatibility contracts, or external kapi-agent review surfaces.

QA / Verification

  • npm test -- test/architecture.test.ts test/worker-dispatch-guard.test.ts test/target-resolution.test.ts test/artifact-list.test.ts — pass; package script runs the full suite shape, 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.
  • Static added-line scan for hardcoded secrets, shell injection, dynamic eval/exec, pickle, and SQL formatting — no matches.
  • Independent reviewer subagent — pass; no security concerns or logic errors.

Residual scan summary:

  • KapiService remains in src/application/workflow-service.ts only as the temporary alias.
  • Test KapiService uses remain in test/service-store.test.ts and test/autoresearch-validation.test.ts.
  • test/architecture.test.ts keeps a negative assertion for absent createLocalKapiService.

Anomalies observed

Symptom Evidence Impact Action
Targeted-looking npm test -- ... still ran the full suite package script is tsx --test test/*.test.ts; output showed 521 tests Slower than a narrow targeted run, but gives broad confidence Reported honestly in QA
Test execution flipped src/cli/kapi-review-cli.ts executable mode git diff -- src/cli/kapi-review-cli.ts showed mode-only churn Unrelated noise risk Restored mode with chmod 644 before commit
Independent reviewer noted context count wording drift reviewer saw 27 test files + docs, not 28 test files No code impact PR body uses corrected count

Risks / Follow-up

  • Follow-up: migrate test/service-store.test.ts.
  • Follow-up: migrate test/autoresearch-validation.test.ts.
  • Follow-up: remove export { WorkflowService as KapiService } after the final compatibility consumers are gone.

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.
  • Current head: 9d40dd4675a7fc517243745382aca10a11c1fbb1.

@devkade
Copy link
Copy Markdown
Owner Author

devkade commented May 17, 2026

@kapi-agent review

Revision explanation for current head 9d40dd4675a7fc517243745382aca10a11c1fbb1:

What changed:

  • Migrated the first batch of small service tests from KapiService compatibility imports/constructors to WorkflowService.
  • Updated docs/product-name-audit.md to narrow remaining compatibility-import work to test/service-store.test.ts and test/autoresearch-validation.test.ts.
  • Kept the temporary service alias in place for the remaining large test clusters.

Why this closes the prior feedback:

  • This slice removes broad small-test dependency on the product-named service alias without mixing in the large clusters or alias removal.
  • Remaining KapiService uses are now concentrated and documented for follow-up slices.

Verification:

  • npm test -- test/architecture.test.ts test/worker-dispatch-guard.test.ts test/target-resolution.test.ts test/artifact-list.test.ts passed; package script ran 521 tests, 510 pass, 11 skipped.
  • npm run check passed.
  • npm run check:unused passed.
  • npm run quality:budgets passed with existing non-failing code_smells=52 warning.
  • git diff --check passed.
  • Independent reviewer subagent passed with no security concerns or logic errors.

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’s original goal is to migrate a bounded first batch of test-only KapiService compatibility imports to the semantic WorkflowService name while leaving larger clusters for follow-up. The semantic scope is narrow: mechanical test import/constructor/type-reference renaming plus an audit doc update. There is no prior kapi-agent review on this head, so no fix ping-pong was required.

What changed

  • Updated 27 small service-related test files to import and instantiate WorkflowService.
  • Left test/service-store.test.ts and test/autoresearch-validation.test.ts on KapiService intentionally for later dedicated slices.
  • Updated docs/product-name-audit.md to document the reduced compatibility-import surface and remaining follow-up work.

Why this is correct

The implementation class already exists as WorkflowService, and KapiService is only a compatibility alias. These changes move tests toward the semantic API without changing runtime behavior, serialized workflow IDs, storage paths, or external kapi-agent/kapi-review integration contracts. The remaining KapiService references are concentrated exactly where the PR body and audit say they should be.

Evidence

  • Verifier gate: PASS — npm ci && npm run verify exited 0.
  • Size gate: PASS — 176 changed lines, below the 200-line review threshold.
  • Revision-explanation status: not required; found.
  • Ilchul review harness gate: PASS — neutral/comment result with 0 blocking findings.
  • Inspected evidence included the changed-file list, diff/stat, current audit doc content, and a direct git grep for remaining KapiService references.

Remaining risks and approval rationale

Remaining risk is limited to follow-up debt: the compatibility alias still exists for the two large test clusters. That is explicitly documented and intentionally out of scope. Approval is justified because the current slice is test-only plus documentation, mechanically consistent, verified by the full project gate, and preserves the artifact/workflow compatibility contract.

Blocking issues

None.

Warnings / risks

  • src/application/workflow-service.ts still exports WorkflowService as KapiService, and test/service-store.test.ts / test/autoresearch-validation.test.ts still consume it. This is acceptable for this slice because the PR documents those as the remaining large clusters.

Suggestions

  • In the follow-up PRs, consider migrating one large cluster at a time and then removing the compatibility alias only after git grep -n "KapiService" confirms no remaining intended consumers.

Looks good

  • The change is tightly scoped and avoids mixing alias migration with source behavior changes.
  • The audit doc remains aligned with implementation state.
  • Remaining compatibility surfaces are explicit rather than hidden.

Verification notes

  • Verifier gate status: PASS.
  • Size gate status: PASS.
  • Revision-explanation status: not required; found.
  • PR evidence reports full verification via npm ci && npm run verify, including tests, type/check gates, unused checks, and quality budgets.

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: APPROVE

Final approval summary

Review journey

The PR’s original goal is to continue issue #209 by reducing internal product-name leakage after the service rename from KapiService to WorkflowService. The semantic scope is intentionally narrow: migrate a first batch of small service tests to the semantic WorkflowService import/constructor while leaving the two large remaining clusters for follow-up. There was no prior kapi-agent review on this PR, so no unresolved kapi-agent feedback remains.

What changed

  • Updated 27 small test files from KapiService to WorkflowService.
  • Updated docs/product-name-audit.md to document that remaining compatibility-import work is limited to:
    • test/service-store.test.ts
    • test/autoresearch-validation.test.ts
  • Left src/application/workflow-service.ts compatibility alias intact for the remaining clusters.

Why this is correct

The code changes are mechanical import/type/constructor renames against the same source module, preserving runtime behavior while reducing dependency on the temporary compatibility alias. The audit now matches the current repository state: KapiService remains only as the alias plus the two documented test clusters and a negative architecture assertion.

Evidence

  • Verifier gate: PASS — npm ci && npm run verify exited 0.
  • Size gate: PASS — 176 changed lines, under the 200-line review threshold.
  • Revision-explanation status: not required, but found.
  • Ilchul harness gate: PASS — neutral/comment, no blocking findings.
  • Inspected evidence:
    • PR diff summary at HEAD
    • docs/product-name-audit.md
    • remaining KapiService references via git grep
    • verifier output and review harness result

Remaining risks and approval rationale

Remaining risk is intentionally deferred compatibility debt in test/service-store.test.ts and test/autoresearch-validation.test.ts. That risk is documented and bounded, and the alias remains in place, so this slice does not break those tests. Approval is justified because the change is low-risk, test-only plus documentation, mechanically consistent, verified, and aligned with the stated migration plan.

Blocking issues

None.

Warnings / risks

  • KapiService compatibility usage still exists in test/service-store.test.ts and test/autoresearch-validation.test.ts; this is documented as follow-up debt.
  • Local verification appears to have left a mode-only working tree change on src/cli/kapi-review-cli.ts, but that mode change is not part of HEAD for this PR.

Suggestions

  • In the next slice, migrate one of the two large remaining clusters at a time to keep review scope similarly bounded.
  • After both clusters are migrated, remove export { WorkflowService as KapiService } and add/adjust a guard to prevent reintroducing compatibility imports.

Looks good

  • The migration is behavior-preserving: same module path, semantic exported class name, no source logic changes.
  • Documentation accurately narrows the remaining compatibility surface.
  • The PR avoids mixing alias removal with test migration, which reduces regression risk.

Verification notes

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


Engine: pi

@devkade devkade merged commit da6bffb into dev May 17, 2026
3 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