Skip to content

test: migrate workflow store imports#217

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

test: migrate workflow store imports#217
devkade merged 1 commit into
devfrom
refactor/issue-209-file-workflow-store-tests-a

Conversation

@devkade
Copy link
Copy Markdown
Owner

@devkade devkade commented May 17, 2026

Summary

  • Migrates the first batch of test fixtures from FileKapiStore to FileWorkflowStore.
  • Leaves the three largest remaining test clusters for follow-up slices to keep this PR reviewable.
  • Does not change source runtime behavior or persisted workflow contracts.

Linked issue

Refs #209

Problem

PR #216 renamed the source-facing workflow store implementation to FileWorkflowStore while retaining FileKapiStore as a temporary compatibility export. Many tests still imported or instantiated the compatibility alias, keeping the product-prefixed store name alive in reusable test code.

A single all-tests migration was larger than the preferred review slice, so this PR migrates the smaller test files first and leaves the three high-count clusters for separate follow-ups.

Options considered

  1. Migrate every remaining test in one PR
    • Pros: removes the compatibility alias faster.
    • Cons: larger diff; mixes broad test fixture churn into one review.
  2. Migrate smaller test files first, leave large clusters for follow-ups
    • Pros: bounded, mechanical, easy to review; reduces residual surface substantially.
    • Cons: temporary alias still needed until remaining clusters migrate.
  3. Leave tests on compatibility alias until KapiService rename
    • Pros: fewer PRs now.
    • Cons: allows stale product-prefixed imports to keep spreading.

Selected approach

Selected option 2.

This PR updates all current test/*.ts files except the three largest remaining clusters:

  • test/service-store.test.ts
  • test/autoresearch-validation.test.ts
  • test/autoresearch-bridge.test.ts

Implementation by file/surface

  • Updated 25 test files to import/instantiate FileWorkflowStore instead of FileKapiStore.
  • No source files changed.
  • No behavior or assertion changes; this is a symbol/import migration only.

Residual scan

After this PR, remaining FileKapiStore references are limited to:

22 test/service-store.test.ts
19 test/autoresearch-validation.test.ts
15 test/autoresearch-bridge.test.ts
1  src/adapters/file-store.ts  # intentional compatibility export

QA / Verification

  • npm test -- test/active-pointer-safety.test.ts test/active-replacement.test.ts test/fail-lane.test.ts test/worker-list.test.ts test/ralph-artifact-contract.test.ts test/deep-interview-completion-gate.test.ts test/worker-dispatch-guard.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.

Anomalies observed

Symptom Evidence Impact Action
Package test script runs full suite even with file args. Command expanded to tsx --test test/*.test.ts ...; 521 tests ran. Stronger coverage, slower run. Reported here.
Verification toggled src/cli/kapi-review-cli.ts executable bit locally again. git status showed mode-only churn after tests/checks. Would pollute PR if committed. Reset mode to 100644; source file not staged.
quality:budgets still warns on code_smells=52. npm run quality:budgets output. Existing non-failing budget warning; unchanged by this test rename. No action in this PR.

Risks / Follow-up

  • Migrate remaining large clusters:
    • test/service-store.test.ts
    • test/autoresearch-validation.test.ts
    • test/autoresearch-bridge.test.ts
  • Remove export { FileWorkflowStore as FileKapiStore } after the final compatibility references are gone.
  • Continue later with the larger KapiService compatibility rename slice.

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.

@devkade
Copy link
Copy Markdown
Owner Author

devkade commented May 17, 2026

@kapi-agent review

Revision explanation for current head 47de9b3ef729258f8e5e141e5a53862d8aecc5ef:

What changed:

  • Migrated 25 smaller test files from FileKapiStore imports/instantiations to FileWorkflowStore.
  • Left the three largest remaining clusters for follow-up slices: test/service-store.test.ts, test/autoresearch-validation.test.ts, and test/autoresearch-bridge.test.ts.
  • No source runtime behavior changed.

Why this closes the prior feedback:

  • PR refactor: rename workflow store surface #216 introduced FileWorkflowStore as the semantic source-facing store and left FileKapiStore only as compatibility.
  • This PR shrinks the compatibility-alias dependency in tests while staying under a bounded review slice.
  • The remaining alias usage is explicitly inventoried for follow-up cleanup.

Verification:

  • npm test -- test/active-pointer-safety.test.ts test/active-replacement.test.ts test/fail-lane.test.ts test/worker-list.test.ts test/ralph-artifact-contract.test.ts test/deep-interview-completion-gate.test.ts test/worker-dispatch-guard.test.ts — pass; package script ran 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 FileKapiStore references after this PR: test/service-store.test.ts, test/autoresearch-validation.test.ts, test/autoresearch-bridge.test.ts, and the intentional compatibility export in src/adapters/file-store.ts.

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 a bounded test-only migration from the temporary FileKapiStore compatibility alias to the renamed FileWorkflowStore. The semantic scope is narrow: import/constructor symbol churn across a first batch of smaller test files, while explicitly leaving the three largest remaining clusters for follow-up slices. There is no prior kapi-agent ping-pong on this PR.

What changed

The PR updates 25 test/*.test.ts files to import and instantiate FileWorkflowStore instead of FileKapiStore. No source runtime files are changed in the PR diff. The compatibility export remains in src/adapters/file-store.ts.

Why this is correct

The renamed store class is the canonical implementation, and FileKapiStore is only a compatibility alias. The changed test call sites continue to construct the same implementation class, so this should not alter runtime behavior, persistence contracts, or assertions. The residual FileKapiStore references match the PR body’s intentional follow-up scope.

Evidence

  • Verifier gate: PASS — npm ci && npm run verify exited 0.
  • Size gate: PASS — 158 changed lines, below the 200-line semantic review threshold.
  • Revision-explanation status: not required; found.
  • Ilchul harness gate: PASS — neutral/comment result, zero blocking findings.
  • Inspected evidence: PR diff, changed file list, representative migrated tests, src/adapters/file-store.ts export, and residual FileKapiStore grep results.

Remaining risks and approval rationale

Remaining risk is limited to the planned follow-up work: test/service-store.test.ts, test/autoresearch-validation.test.ts, and test/autoresearch-bridge.test.ts still use the compatibility alias. That is explicitly documented and does not block this bounded migration. Approval is justified because the change is mechanical, test-only, verified, and does not weaken artifact/workflow contract behavior.

Blocking issues

None.

Warnings / risks

  • The compatibility alias still remains intentionally in src/adapters/file-store.ts, with remaining references in the three deferred large test clusters. This is acceptable for this slice but should be removed after the follow-up migrations.
  • The local review workspace shows a mode-only modification on src/cli/kapi-review-cli.ts; it does not appear in the PR diff, but it is worth ensuring it is not accidentally committed in a later update.

Suggestions

  • In the follow-up PRs, consider migrating one deferred cluster at a time and removing export { FileWorkflowStore as FileKapiStore } only after the final references are gone.
  • If possible, add a final grep/check in the alias-removal PR to prevent new FileKapiStore imports from reappearing.

Looks good

  • The PR keeps the change test-only and avoids mixing source behavior changes into a naming migration.
  • The residual scan in the PR body matches the local grep shape: only the deferred clusters plus the intentional compatibility export remain.
  • The author documented verification, anomalies, and follow-up scope clearly.

Verification notes

  • Verifier gate status: PASS — npm ci && npm run verify completed successfully.
  • Size gate status: PASS — 158 changed lines, test-only mechanical migration.
  • Revision-explanation status: not required for this opened PR; explanation was present in the PR body.
  • Ilchul review harness: PASS as gate evidence, with no blocking findings.
  • Local inspection confirmed no source runtime behavior changes in the PR diff.

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 original goal is to continue the FileKapiStoreFileWorkflowStore semantic rename by migrating a bounded batch of tests away from the temporary compatibility alias. The semantic scope is narrow: 25 test files only, with import/constructor symbol substitutions and no runtime source behavior changes. There is no prior kapi-agent review on this PR, and the author provided a current-head explanation plus follow-up inventory.

What changed

This PR updates test imports and instantiations from FileKapiStore to FileWorkflowStore across 25 smaller test/*.test.ts files. The remaining compatibility alias usage is intentionally left in:

  • test/service-store.test.ts
  • test/autoresearch-validation.test.ts
  • test/autoresearch-bridge.test.ts
  • src/adapters/file-store.ts compatibility export

Why this is correct

FileKapiStore is exported as an alias of FileWorkflowStore, so the migrated tests continue exercising the same implementation while reducing dependence on the old product-prefixed name. I inspected the diff and confirmed it is mechanical symbol migration rather than assertion or behavior churn.

Evidence

Verifier gate: PASS. Size gate: PASS, 158 changed lines under the 200-line semantic review threshold. Revision-explanation status: not required, but found. Ilchul harness gate: PASS with low-risk profile and no blocking findings. Verified evidence includes npm ci && npm run verify passing, covering tests, type/check gates, unused checks, and quality budgets. I also inspected the local diff/stat and remaining FileKapiStore references.

Remaining risks and approval rationale

Remaining risk is limited to temporary alias usage in the three intentionally deferred large test clusters and the compatibility export. That is explicitly documented and appropriate for a follow-up slice. Approval is justified because this PR is low-risk, tests-only, mechanically consistent, verified, and preserves the artifact/workflow contract.

Blocking issues

None.

Warnings / risks

  • The compatibility alias remains in src/adapters/file-store.ts and three larger test clusters. This is acceptable for this PR, but should not become a long-lived migration endpoint.

Suggestions

  • In the follow-up PRs, consider removing the compatibility export only after the remaining test clusters and any source/runtime callers are fully migrated.
  • Keep the residual scan in future PR bodies until the alias is removed; it makes review scope easy to validate.

Looks good

  • The diff is consistently limited to FileKapiStoreFileWorkflowStore in tests.
  • No source runtime behavior or persisted workflow contract changes are included.
  • The remaining alias references match the PR body’s stated follow-up inventory.
  • Verification coverage is strong for a tests-only mechanical rename.

Verification notes

Verifier gate status: PASS — npm ci && npm run verify exited 0.
Size gate status: PASS — 158 changed lines, below threshold.
Revision-explanation status: not required, found.
Ilchul review harness: PASS — neutral/comment output, low-risk profile, no blocking findings.
Local inspection: reviewed diff/stat and residual FileKapiStore references; no blocking correctness, safety, maintainability, or workflow-contract issues found.


Engine: pi

@devkade devkade merged commit 9b5d368 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