refactor: rename workflow service source surface#219
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’s original goal is a bounded refactor to rename the source-facing workflow orchestration service from KapiService to WorkflowService while preserving compatibility. The semantic scope is narrow: application service export, local factory typing/construction, presentation-layer service types, runtime probe imports, and the product-name audit note. There was no prior kapi-agent review ping-pong for this head.
What changed
Key paths inspected:
src/application/workflow-service.tssrc/adapters/workflow-service-factory.tssrc/presentation/*autoresearch.shdocs/product-name-audit.md
The implementation now exports WorkflowService as the primary class and keeps export { WorkflowService as KapiService }; as a temporary compatibility bridge.
Why this is correct
The runtime and presentation source-facing call sites now depend on WorkflowService, while existing tests/importers using KapiService remain compatible. This keeps the refactor non-breaking and matches the stated bounded migration strategy.
Evidence
- Verifier gate: PASS —
npm ci && npm run verifyexited 0. - Size gate: PASS — 55 changed lines, below the 200-line semantic review threshold.
- Ilchul harness gate: PASS — neutral/comment, 0 blocking findings.
- Revision explanation: not required; found.
- Inspected diff and searched for remaining
KapiService/WorkflowServicereferences.
Remaining risks and approval rationale
Remaining risk is limited to temporary compatibility debt: many tests still import KapiService, so the alias cannot be removed yet. This is explicitly documented as follow-up. Approval is justified because the compatibility bridge preserves behavior, verification passes, and no blocking correctness or safety issue remains.
Blocking issues
- None.
Warnings / risks
docs/product-name-audit.mdstill has one slightly stale sentence in the “Application service filenames” row saying exported class/function names are “intentionally left for a smaller follow-up compatibility slice.” The service class has now been renamed, so that row could be tightened in a follow-up to avoid audit drift.
Suggestions
- Add or extend an architecture test later to assert that production source imports use
WorkflowServiceand only the compatibility export/tests retainKapiService. - Migrate test imports from
KapiServicetoWorkflowServicein a separate bounded PR, then remove the compatibility alias.
Looks good
- The compatibility export is a safe migration bridge.
- Runtime probes in
autoresearch.shwere updated consistently. - Factory return type and construction now use
WorkflowService. - Presentation types are routed through semantic service naming without changing runtime behavior.
Verification notes
- Verifier gate: PASS —
npm ci && npm run verifyexited 0. - Size gate: PASS — 55 changed lines, small bounded refactor.
- Revision-explanation status: not required; found.
- PR evidence includes passing tests, type/check gates, unused checks, and quality budgets.
Engine: pi
There was a problem hiding this comment.
kapi-agent review
Verdict: APPROVE
Final approval summary
Review journey
The PR goal is to continue issue #209/#205 product-name cleanup by renaming the source-facing workflow orchestration service from KapiService to WorkflowService. The semantic scope is narrow: application service class naming, factory/presentation/runtime probe imports, and audit documentation. There was no prior kapi-agent blocking review on this head, and the author provided a clear current-head explanation.
What changed
src/application/workflow-service.ts: primary class is nowWorkflowService; temporaryKapiServicealias remains.src/adapters/workflow-service-factory.ts: factory now returns/constructsWorkflowService.src/presentation/*: service type surfaces now referenceWorkflowService.autoresearch.sh: runtime probes instantiateWorkflowService.docs/product-name-audit.md: records the renamed service surface and remaining compatibility alias.
Why this is correct
The implementation file was already semantically named workflow-service.ts, and the class now matches that boundary. Runtime and presentation source surfaces no longer depend on the product-prefixed service name. Keeping export { WorkflowService as KapiService } is a safe compatibility bridge for existing tests/importers and keeps this slice bounded.
Evidence
- Verifier gate: PASS.
- Size gate: PASS, 55 changed lines under the semantic threshold.
- Revision-explanation status: not required, but found.
- Ilchul review harness: PASS / neutral, no blocking findings.
npm ci && npm run verifypassed.- Inspected the changed service, factory, presentation imports, runtime probe updates, and audit doc.
Remaining risks and approval rationale
Remaining risk is limited to the intentional temporary KapiService alias and unmigrated test imports. This is documented as follow-up and does not break behavior. Approval is justified because the change is small, mechanically consistent, verified, and preserves compatibility.
Blocking issues
None.
Warnings / risks
src/application/workflow-service.ts: the temporaryKapiServicecompatibility export should be removed once remaining tests/importers migrate.docs/product-name-audit.md: the “Application service filenames” row still says exported class/function names are intentionally left for a smaller follow-up compatibility slice. That wording is now slightly stale for the service class, though not blocking.
Suggestions
- In a follow-up, migrate test imports from
KapiServicetoWorkflowServiceand then remove the alias. - Tighten the audit doc wording so it distinguishes the already-renamed service class from any remaining compatibility aliases.
Looks good
- The rename is consistent across runtime-facing source surfaces.
- Compatibility is preserved for existing imports.
- The PR avoids a broad product-name replacement and keeps persisted/external
kapi-*contracts out of scope. - The audit doc captures the remaining migration state.
Verification notes
Verifier gate status: PASS (npm ci && npm run verify exited 0).
Size gate status: PASS (55 changed lines < 200).
Revision-explanation status: not required, found.
Ilchul harness gate: PASS / neutral with no blocking findings.
Engine: pi
f4dbb83 to
37db397
Compare
|
@kapi-agent review Revision explanation for current head What changed since prior approved head:
Why this closes the prior feedback:
Verification:
|
There was a problem hiding this comment.
kapi-agent review
Verdict: REQUEST_CHANGES
Blocking issues
- Revision explanation gate failed. This PR was synchronized after prior kapi-agent approvals, but no current revision explanation was found for head
37db3973394c4913a30c71ddf11d1ea380e83670. Per review policy, please add a short author comment explaining:- what changed since the last approved head,
- why the change was needed,
- why it closes or preserves the prior review conclusions,
- what verification was rerun.
Warnings / risks
- No code-level blocking issue found from the provided diff and gate evidence.
src/application/workflow-service.tskeepsexport { WorkflowService as KapiService };as a compatibility bridge. That is reasonable for this bounded slice, but it should remain tracked as temporary debt.
Suggestions
- In the revision response, include the current head SHA to make the review trail unambiguous.
- Keep the follow-up migration/removal of the
KapiServicealias scoped and separately reviewed.
Looks good
- The source-facing rename from
KapiServicetoWorkflowServiceis narrow and consistent across:src/application/workflow-service.tssrc/adapters/workflow-service-factory.tsautoresearch.shdocs/product-name-audit.md
- The compatibility export preserves existing importers while allowing the source surface to move toward semantic naming.
- Documentation records the remaining compatibility alias and follow-up direction.
Verification notes
- Verifier gate status: FAIL overall, because the revision explanation gate failed.
- Size gate status: PASS — 57 changed lines, below the 200-line semantic review threshold.
- Revision-explanation status: FAIL — required after synchronize, not found for the current head.
- Test/CI evidence:
npm ci && npm run verifypassed; Ilchul review harness passed with neutral/comment verdict and no blocking findings.
Engine: pi
There was a problem hiding this comment.
kapi-agent review
Verdict: APPROVE
Final approval summary
Review journey
The original goal is a bounded product-name cleanup refactor: rename the source-facing workflow orchestration service from KapiService to WorkflowService while preserving temporary compatibility. The semantic scope remains narrow: application service naming, factory/presentation/runtime probe imports, and the product-name audit documentation. This head follows prior kapi-agent approval and addresses the noted stale audit-doc wording.
What changed
Current revision tightens docs/product-name-audit.md so it now states that WorkflowService is the primary exported service class and KapiService is only a temporary compatibility alias. The implementation paths remain:
src/application/workflow-service.tssrc/adapters/workflow-service-factory.tssrc/presentation/*autoresearch.shdocs/product-name-audit.md
Why this is correct
The implementation keeps a non-breaking compatibility export while moving source-facing call sites to the semantic WorkflowService name. The revised audit doc now matches the actual code state and no longer implies the service class rename is future work.
Evidence
- Verifier gate: PASS —
npm ci && npm run verifyexited 0. - Size gate: PASS — 57 changed lines, below the 200-line review threshold.
- Ilchul review harness gate: PASS — neutral/comment, 0 blocking findings.
- Revision explanation status: found; the author explained the post-approval doc tightening.
- Inspected local HEAD diff and representative files including
workflow-service.ts,workflow-service-factory.ts, presentation type changes,autoresearch.sh, anddocs/product-name-audit.md.
Remaining risks and approval rationale
Remaining risk is limited to the intentional temporary KapiService compatibility alias, which is documented as follow-up work. No blocking correctness, safety, workflow-contract, or maintainability issue remains, and the verification evidence is sufficient for this bounded refactor.
Blocking issues
None.
Warnings / risks
src/application/workflow-service.ts: the temporaryKapiServicealias should be removed once remaining tests/importers migrate, to avoid the compatibility bridge becoming permanent.- Local review checkout shows a mode-only modification on
src/cli/kapi-review-cli.ts, but it is not part of the reviewed HEAD commit.
Suggestions
- In a follow-up slice, migrate remaining internal compatibility imports/tests away from
KapiService, then remove the alias and update the residual audit accordingly.
Looks good
- The primary implementation name now matches the semantic file boundary.
- Presentation, factory, and runtime probe surfaces use
WorkflowService. - The audit doc now accurately distinguishes completed rename work from the remaining compatibility alias.
Verification notes
- Verifier gate: PASS —
npm ci && npm run verifyexited 0. - Size gate: PASS — 57 changed lines < 200.
- Revision-explanation status: found.
- PR/local evidence: full verify passed; author also reported
git diff --check,npm run check,npm run check:unused, andnpm run quality:budgetspassing.
Engine: pi
Summary
KapiServicetoWorkflowService.WorkflowService.KapiServicecompatibility export so existing tests/importers can migrate in follow-up slices.Linked issue
Refs #209
Problem
After the store cleanup, the main remaining reusable internal product-prefixed service symbol was
KapiService. The implementation file is already semantic (workflow-service.ts), and the class represents generic workflow orchestration rather than a Kapi-specific product surface.A full test migration would be much larger than a small review slice, so this PR renames the source-facing implementation and runtime/presentation call sites first while retaining a temporary compatibility export.
Implementation
src/application/workflow-service.tsexport class KapiServicetoexport class WorkflowService.export { WorkflowService as KapiService };as a temporary compatibility bridge.src/adapters/workflow-service-factory.tsWorkflowService.src/presentation/*WorkflowService.autoresearch.shWorkflowService.docs/product-name-audit.mdResidual scan
QA / Verification
npm run check— pass.npm run check:unused— pass.npm test -- test/architecture-boundaries.test.ts test/run-contract-prompt-renderer.test.ts test/tool-core.test.ts test/kapi-agent-tools.test.ts— pass; package script ran the full suite: 521 tests, 510 pass, 11 skipped.npm run quality:budgets— pass with existing non-failingcode_smells=52warning.git diff --check— pass.Anomalies observed
tsx --test test/*.test.ts ...; 521 tests ran.src/cli/kapi-review-cli.tsexecutable bit locally again.git statusshowed mode-only churn after tests/checks.100644; source file not staged.quality:budgetsstill warns oncode_smells=52.npm run quality:budgetsoutput.Risks / Follow-up
KapiServicetoWorkflowServicein bounded follow-up slices.KapiServicecompatibility export once all internal imports are migrated.kapi-agentremain intentionally out of scope.kapi-agent review
@kapi-agent review.