test: remove workflow store compatibility imports#218
Conversation
|
@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
This PR completes a narrow rename cleanup for issue #209. The semantic scope is limited to removing the legacy FileKapiStore compatibility surface and migrating the remaining tests to FileWorkflowStore. There was no prior kapi-agent review on this head, so no fix ping-pong was required.
What changed
src/adapters/file-store.ts: removedexport { FileWorkflowStore as FileKapiStore };.test/service-store.test.ts,test/autoresearch-validation.test.ts,test/autoresearch-bridge.test.ts: replaced remainingFileKapiStoreimports/instantiations withFileWorkflowStore.
Why this is correct
The implementation class is already FileWorkflowStore, and the temporary compatibility export only preserved the old name. The tests now exercise the canonical name directly, and the residual scan shows no remaining FileKapiStore references under the intended source/test surface.
Evidence
- Verifier gate: PASS.
- Size gate: PASS, 114 changed lines under threshold.
- Revision-explanation status: not required; explanation present in PR body.
- Ilchul harness: PASS/neutral, no blocking findings.
- CI/local evidence supplied by gate:
npm ci && npm run verifypassed. - Additional inspection: checked package metadata and searched for remaining
FileKapiStorereferences insrcandtest; none found.
Remaining risks and approval rationale
Remaining risk is low: removing an exported alias can break out-of-tree imports, but this package is marked private and the change is aligned with the stated cleanup issue. The touched code is almost entirely tests plus removal of the compatibility alias, with full verification passing. Approval is justified.
Blocking issues
None.
Warnings / risks
src/adapters/file-store.ts: removing the compatibility export is intentionally breaking for any local/out-of-tree code importingFileKapiStore. Given the package is private and the PR explicitly completes the rename cleanup, this is acceptable.
Suggestions
- Consider a follow-up scan outside
src/testif local extension examples or docs can import internal adapters, though I did not see evidence that this is required for this PR.
Looks good
- The change is mechanically consistent: all migrated tests use
FileWorkflowStore. - The old compatibility export is removed only after the remaining in-repo consumers were migrated.
- Verification coverage is strong for a rename cleanup: full tests, typecheck, unused checks, quality budgets, and residual symbol scan.
Verification notes
- Verifier gate status: PASS —
npm ci && npm run verifyexited 0. - Size gate status: PASS — 114 changed lines, below the semantic review threshold.
- Revision-explanation status: not required; found in PR body.
- Ilchul review harness gate: PASS/neutral, no blocking findings.
- PR evidence includes successful targeted/full test execution,
git diff --check,npm run check,npm run check:unused,npm run quality:budgets, andrg -n '\bFileKapiStore\b' test src/adapters/file-store.tswith no matches.
Engine: pi
There was a problem hiding this comment.
kapi-agent review
Verdict: APPROVE
Final approval summary
Review journey
This PR completes a narrow store-surface rename cleanup for issue #209. The semantic scope is bounded: migrate the remaining test imports/instantiations from FileKapiStore to FileWorkflowStore, then remove the temporary compatibility export.
What changed
src/adapters/file-store.ts: removedexport { FileWorkflowStore as FileKapiStore };.test/service-store.test.ts,test/autoresearch-validation.test.ts, andtest/autoresearch-bridge.test.ts: migrated remainingFileKapiStorereferences toFileWorkflowStore.
Why this is correct
The implementation already exposes FileWorkflowStore as the canonical store class. The removed alias was temporary compatibility surface, and the updated tests now exercise the canonical name directly. A residual scan shows no remaining FileKapiStore references in src or test, so the rename surface is internally consistent.
Evidence
- Verifier gate: PASS —
npm ci && npm run verifyexited 0. - Size gate: PASS — 114 changed lines, below the 200-line review threshold; mostly test-only rename updates.
- Ilchul harness gate: PASS — neutral/comment result with 0 blocking findings.
- Revision-explanation status: not required, found.
- Inspected files:
src/adapters/file-store.ts, changed test imports/usages,package.json, and repository-wide symbol scan.
Remaining risks and approval rationale
Remaining risk is low and limited to external consumers importing FileKapiStore directly from an internal adapter path. This package is marked private, and the PR intentionally removes a temporary compatibility alias after internal migration. Approval is justified because the change is narrow, verification passes, and no blocking correctness, safety, or workflow-contract issue remains.
Blocking issues
None.
Warnings / risks
src/adapters/file-store.ts: removing the alias is a breaking change for any out-of-tree/internal scripts that still importFileKapiStore. Given the package is private and the PR goal is to complete the compatibility cleanup, this is acceptable.
Suggestions
- Consider documenting in the linked issue or follow-up checklist that
FileKapiStorecompatibility is now fully removed, so future rename work can focus on the remainingKapiServicenaming surface.
Looks good
- The compatibility export removal is minimal and matches the stated migration goal.
- Tests were updated to use the canonical
FileWorkflowStorename directly. - The residual scan for
FileKapiStoreacrosssrcandtestsupports that the internal rename is complete for this symbol. - No generated files or sensitive paths are involved.
Verification notes
- Verifier gate status: PASS —
npm ci && npm run verifyexited 0. - Size gate status: PASS — 114 changed lines, below threshold; mostly test rename churn.
- Revision-explanation status: not required for this review, but an explanation was found.
- PR evidence includes full test/check/unused/quality verification and a no-match scan for
FileKapiStore.
Engine: pi
Summary
FileKapiStoretest clusters toFileWorkflowStore.FileKapiStorecompatibility export fromsrc/adapters/file-store.ts.FileKapiStorereferences intest/orsrc/adapters/file-store.ts.Linked issue
Refs #209
Problem
The previous store rename slices moved the source-facing implementation to
FileWorkflowStoreand migrated the smaller test files. Three large test clusters and the temporary compatibility export still kept the oldFileKapiStorename alive.Implementation
test/service-store.test.tsFileKapiStoreimports/instantiations withFileWorkflowStore.test/autoresearch-validation.test.tsFileKapiStoreimports/instantiations withFileWorkflowStore.test/autoresearch-bridge.test.tsFileKapiStoreimports/instantiations withFileWorkflowStore.src/adapters/file-store.tsexport { FileWorkflowStore as FileKapiStore };.Residual scan
QA / Verification
npm test -- test/service-store.test.ts test/autoresearch-validation.test.ts test/autoresearch-bridge.test.ts— pass; package script ran the full suite: 521 tests, 510 pass, 11 skipped.git diff --check— pass after fixing one trailing blank line.npm run check— pass.npm run check:unused— pass.npm run quality:budgets— pass with existing non-failingcode_smells=52warning.rg -n '\bFileKapiStore\b' test src/adapters/file-store.ts— no matches.Anomalies observed
git diff --checkfound a trailing blank line at EOF after alias removal.src/adapters/file-store.ts:391: new blank line at EOF.git diff --checksuccessfully.tsx --test test/*.test.ts ...; 521 tests ran.quality:budgetsstill warns oncode_smells=52.npm run quality:budgetsoutput.Risks / Follow-up
KapiServicenaming surface and intentional external/history literals.kapi-agent review
@kapi-agent review.