411 lines
17 KiB
Markdown
411 lines
17 KiB
Markdown
# X-Financial Duplicate Code Refactor Plan
|
|
|
|
> **For agentic workers:** REQUIRED SUB-SKILL: Use superpowers:subagent-driven-development or superpowers:executing-plans for multi-task execution. Steps use checkbox (`- [ ]`) syntax for tracking.
|
|
|
|
**Goal:** Reduce duplicated business logic, renderer helpers, protocol constants, and test fixtures without changing existing behavior.
|
|
|
|
**Architecture:** Start with low-risk pure helpers, then move toward business contract consolidation. Each round must add or preserve regression tests before production code changes, and backend validation must run inside `x-financial-main`.
|
|
|
|
**Tech Stack:** Vue 3, Vite, Node test runner, FastAPI, SQLAlchemy, pytest, Docker Compose.
|
|
|
|
---
|
|
|
|
## Scope
|
|
|
|
This document records the duplicate-code audit from 2026-06-23 and defines a staged cleanup path. The first implementation slice is intentionally small: extract shared frontend conversation rendering helpers used by `markdown.js` and `aiConversationHtmlRenderer.js`.
|
|
|
|
The following areas are in scope:
|
|
|
|
- Frontend AI markdown / HTML trusted block normalization.
|
|
- Frontend reimbursement review panel model duplication.
|
|
- Workbench AI composer / attachment strip duplication.
|
|
- Backend application gate, fact extraction, amount/date/location parsing.
|
|
- Backend platform risk context helper duplication.
|
|
- Cross-layer status, expense type, document type, and risk-level taxonomy drift.
|
|
- Test helper duplication for DB sessions, FastAPI client overrides, and OCR fakes.
|
|
|
|
The following areas are out of scope for the first implementation slice:
|
|
|
|
- Changing application submission behavior.
|
|
- Changing reimbursement association decision flow.
|
|
- Changing API response shapes.
|
|
- Editing unrelated notification top bar changes already present in the worktree.
|
|
|
|
## Findings
|
|
|
|
### P0: Frontend Trusted HTML And Conversation Text Helpers
|
|
|
|
`web/src/utils/markdown.js` and `web/src/utils/aiConversationHtmlRenderer.js` both implement:
|
|
|
|
- `ALLOWED_COLON_HEADING_TITLES`
|
|
- `BUSINESS_FIELD_LABELS`
|
|
- `TRUSTED_HTML_ALLOWED_TAGS`
|
|
- `TRUSTED_HTML_ALLOWED_ATTRS`
|
|
- `splitColonHeadingLine`
|
|
- `normalizeBusinessFieldLine`
|
|
- `hasOnlyTrustedHtmlTags`
|
|
- `sanitizeTrustedHtmlBlock`
|
|
- `extractTrustedHtmlBlocks`
|
|
- `restoreTrustedHtmlBlocks`
|
|
|
|
Plan:
|
|
|
|
- [x] Create `web/src/utils/conversationTrustedHtml.js`.
|
|
- [x] Move trusted HTML sanitizing and business-line normalization into the helper.
|
|
- [x] Keep renderer-specific output differences in each renderer.
|
|
- [x] Verify both markdown and AI conversation renderers still preserve valid trusted document cards and reject unsafe trusted HTML.
|
|
|
|
Expected benefit:
|
|
|
|
- One XSS whitelist.
|
|
- One business field normalization rule.
|
|
- Less drift between AI workbench and reimbursement assistant rendering.
|
|
|
|
### P0: Reimbursement Review Panel Model Duplication
|
|
|
|
`web/src/views/scripts/travelReimbursementCreateReviewModel.js` and `web/src/views/scripts/travelReimbursementReviewPanelModel.js` duplicate review scope, fact cards, risk item mapping, risk conversation text, and message cleanup.
|
|
|
|
Plan:
|
|
|
|
- [x] Choose `travelReimbursementReviewPanelModel.js` as the shared model.
|
|
- [x] Convert create-view imports to the shared model, or make the create-view module a thin compatibility re-export.
|
|
- [x] Add behavior tests for exported review helpers before deleting duplicate code.
|
|
|
|
Expected benefit:
|
|
|
|
- One risk item mapping.
|
|
- One review fact card model.
|
|
- Lower risk when changing reimbursement review copy or drawer behavior.
|
|
|
|
### P0: Workbench AI Composer And File Strip Template Duplication
|
|
|
|
`web/src/components/business/PersonalWorkbenchAiMode.template.html` keeps two near-identical composer forms and two near-identical selected-file strips for the welcome and inline conversation states. `web/src/components/business/workbench-ai/WorkbenchAiComposer.vue` and `web/src/components/business/workbench-ai/WorkbenchAiFileStrip.vue` already exist, but the main template still duplicates the markup.
|
|
|
|
Plan:
|
|
|
|
- [x] Reuse `WorkbenchAiComposer.vue` for both welcome and inline composer surfaces.
|
|
- [x] Reuse `WorkbenchAiFileStrip.vue` for both welcome and inline selected-file strips.
|
|
- [x] Keep the parent runtime API stable by passing a proxied runtime object into shared components.
|
|
- [x] Preserve OCR state display in the shared file-strip component.
|
|
- [x] Keep input focus behavior by exposing an explicit assistant input ref setter.
|
|
|
|
Expected benefit:
|
|
|
|
- One composer markup surface.
|
|
- One selected-file/OCR badge markup surface.
|
|
- Lower maintenance cost when upload, date picker, lock state, or send-button behavior changes.
|
|
|
|
### P1: Application Gate And Fact Extraction
|
|
|
|
Backend application flow repeats checks across `user_agent_application.py`, `steward_planner.py`, `orchestrator.py`, `ontology_detection.py`, and `ontology_extraction.py`.
|
|
|
|
Plan:
|
|
|
|
- [ ] Extract application intent / context gate helpers.
|
|
- [ ] Extract application fact resolver for date, location, reason, amount, transport, and expense type.
|
|
- [ ] Route UserAgent, StewardPlanner, and Orchestrator through the same helpers.
|
|
- [ ] Preserve existing application vs reimbursement stage boundaries.
|
|
|
|
Expected benefit:
|
|
|
|
- Fewer mismatches between button actions and text-input actions.
|
|
- Less chance of direct submit re-entering slow `/orchestrator/run` paths.
|
|
- More consistent missing-field prompts.
|
|
|
|
### P1: Backend Parsing And Risk Context Utilities
|
|
|
|
Several backend modules repeat city extraction, document field lookup, item-id dedupe, Decimal conversion, endpoint normalization, and JSON error parsing.
|
|
|
|
Plan:
|
|
|
|
- [ ] Extract platform risk context helpers for item-id and document field utilities.
|
|
- [ ] Reuse existing amount utilities before adding new regex parsing.
|
|
- [ ] Share model connectivity URL/header/error helpers between RAG runtime and connectivity checks.
|
|
- [ ] Cache sorted travel-policy city names per policy snapshot.
|
|
|
|
Expected benefit:
|
|
|
|
- Less CPU churn in repeated risk/OCR loops.
|
|
- Fewer provider connectivity behavior differences.
|
|
- Easier review of platform-risk regressions.
|
|
|
|
### P1: Cross-Layer Taxonomy And Protocol Constants
|
|
|
|
Status labels, expense types, document types, risk levels, API paths, and snake_case/camelCase mappings are repeated across backend schemas, frontend services, and tests.
|
|
|
|
Plan:
|
|
|
|
- [ ] Establish read-only contract baseline from OpenAPI export.
|
|
- [ ] Export status / approval-stage registry to frontend constants.
|
|
- [ ] Consolidate expense type, document type, and risk-level taxonomy.
|
|
- [ ] Move high-churn API path and payload builders into shared frontend test helpers.
|
|
|
|
Expected benefit:
|
|
|
|
- Fewer display inconsistencies.
|
|
- Easier API evolution.
|
|
- Less brittle source-string tests.
|
|
|
|
### P2: Test Fixture Duplication
|
|
|
|
`server/tests` repeatedly defines `build_session`, `build_session_factory`, `override_db`, `build_client`, and OCR fake functions.
|
|
|
|
Plan:
|
|
|
|
- [ ] Add backend test fixtures in `server/tests/conftest.py`.
|
|
- [ ] Move OCR fake builders into a small test helper.
|
|
- [ ] Migrate tests in batches, one behavior area at a time.
|
|
|
|
Expected benefit:
|
|
|
|
- Less boilerplate in large test files.
|
|
- Easier targeted regression coverage before service refactors.
|
|
|
|
## First Slice Execution Plan
|
|
|
|
### Task 1: Lock Shared Renderer Helper Behavior
|
|
|
|
**Files:**
|
|
|
|
- Create: `web/tests/conversation-trusted-html.test.mjs`
|
|
- Create: `web/src/utils/conversationTrustedHtml.js`
|
|
- Modify: `web/src/utils/markdown.js`
|
|
- Modify: `web/src/utils/aiConversationHtmlRenderer.js`
|
|
|
|
Steps:
|
|
|
|
- [x] Add a failing Node test that imports `conversationTrustedHtml.js`.
|
|
- [x] Assert valid trusted document-card HTML is preserved through placeholder extraction and restore.
|
|
- [x] Assert unsafe tags, event handlers, and non-document hrefs are rejected.
|
|
- [x] Assert colon headings and business field lines normalize outside fenced code blocks.
|
|
- [x] Run the new test and confirm it fails because the helper does not exist.
|
|
- [x] Implement the helper with pure functions only.
|
|
- [x] Refactor both renderers to use the helper.
|
|
- [x] Run targeted renderer tests.
|
|
- [x] Run `npm --prefix web run build`.
|
|
|
|
Validation commands:
|
|
|
|
```bash
|
|
node --test web/tests/conversation-trusted-html.test.mjs
|
|
node --test web/tests/ai-conversation-html-renderer.test.mjs web/tests/travel-reimbursement-review-drawer-switch.test.mjs
|
|
npm --prefix web run build
|
|
```
|
|
|
|
## Third Slice Execution Plan
|
|
|
|
### Task 3: Reuse Workbench AI Composer Components
|
|
|
|
**Files:**
|
|
|
|
- Create: `web/tests/workbench-ai-composer-components.test.mjs`
|
|
- Modify: `web/src/components/business/PersonalWorkbenchAiMode.vue`
|
|
- Modify: `web/src/components/business/PersonalWorkbenchAiMode.template.html`
|
|
- Modify: `web/src/components/business/workbench-ai/WorkbenchAiFileStrip.vue`
|
|
- Modify: `web/src/composables/workbenchAiMode/usePersonalWorkbenchAiMode.js`
|
|
|
|
Steps:
|
|
|
|
- [x] Add a failing Node test that expects the main template to use `WorkbenchAiComposer` and `WorkbenchAiFileStrip`.
|
|
- [x] Assert the shared file strip preserves OCR state badges.
|
|
- [x] Assert the runtime exposes an input ref setter for the shared composer.
|
|
- [x] Run the new test and confirm it fails on the duplicated template.
|
|
- [x] Pass a proxied runtime object from `PersonalWorkbenchAiMode.vue` into shared components.
|
|
- [x] Replace duplicate composer and file-strip markup in the external template.
|
|
- [x] Add OCR badge markup to `WorkbenchAiFileStrip.vue`.
|
|
- [x] Run targeted workbench AI tests.
|
|
- [x] Run `npm --prefix web run build`.
|
|
|
|
Validation commands:
|
|
|
|
```bash
|
|
node --test web/tests/workbench-ai-composer-components.test.mjs
|
|
node --test web/tests/workbench-ai-composer-components.test.mjs web/tests/workbench-ai-mode-switch.test.mjs web/tests/workbench-ai-mode-expense-scene-action.test.mjs
|
|
npm --prefix web run build
|
|
```
|
|
|
|
## Remaining Execution Plan
|
|
|
|
The remaining work is intentionally split into bounded slices. Each slice extracts shared code without changing user-facing flow, API response shape, or submission semantics.
|
|
|
|
### Task 4: Frontend Application Gate Helpers
|
|
|
|
**Files:**
|
|
|
|
- Create: `web/src/composables/workbenchAiMode/workbenchAiApplicationGateModel.js`
|
|
- Create: `web/tests/workbench-ai-application-gate-model.test.mjs`
|
|
- Modify: `web/src/composables/workbenchAiMode/usePersonalWorkbenchAiMode.js`
|
|
- Modify: `web/src/composables/workbenchAiMode/useWorkbenchAiApplicationPreviewFlow.js`
|
|
|
|
Steps:
|
|
|
|
- [x] Add a failing Node test for reimbursement creation intent, submit/save text action resolution, and orphan preview detection.
|
|
- [x] Move pure gate predicates into `workbenchAiApplicationGateModel.js`.
|
|
- [x] Replace inline copies in personal workbench and application-preview flow.
|
|
- [x] Run targeted workbench AI application tests.
|
|
|
|
Validation commands:
|
|
|
|
```bash
|
|
node --test web/tests/workbench-ai-application-gate-model.test.mjs
|
|
node --test web/tests/workbench-ai-application-gate-model.test.mjs web/tests/workbench-ai-mode-switch.test.mjs web/tests/workbench-ai-action-router.test.mjs
|
|
```
|
|
|
|
### Task 5: Backend Application Fact Resolver
|
|
|
|
**Files:**
|
|
|
|
- Create: `server/src/app/services/application_fact_resolver.py`
|
|
- Create: `server/tests/test_application_fact_resolver.py`
|
|
- Modify: `server/src/app/services/steward_planner.py`
|
|
|
|
Steps:
|
|
|
|
- [x] Add failing pytest coverage for time, location, reason, transport, and task-type inference.
|
|
- [x] Extract pure resolver helpers that preserve existing planner output.
|
|
- [x] Route `StewardPlannerService` extraction wrappers through the resolver.
|
|
- [x] Run targeted planner/fact tests inside the active app container.
|
|
|
|
Validation commands:
|
|
|
|
```bash
|
|
docker exec -w /app -e SERVER_VENV_DIR=/tmp/x-financial-server-venv x-financial-main /tmp/x-financial-server-venv/bin/pytest -q server/tests/test_application_fact_resolver.py server/tests/test_steward_planner.py
|
|
```
|
|
|
|
### Task 6: Backend Platform Risk Context Utilities
|
|
|
|
**Files:**
|
|
|
|
- Modify: `server/src/app/services/expense_claim_platform_context_tools.py`
|
|
- Create: `server/tests/test_expense_claim_platform_context_tools.py`
|
|
- Modify: `server/src/app/services/expense_claim_platform_route_risk.py`
|
|
- Modify: `server/src/app/services/expense_claim_platform_risk.py`
|
|
|
|
Steps:
|
|
|
|
- [x] Add failing pytest coverage for context city extraction, item-id dedupe, and text-value dedupe helpers.
|
|
- [x] Add pure helper functions in `expense_claim_platform_context_tools.py`.
|
|
- [x] Route route-risk and platform-risk consumers through the shared helpers.
|
|
- [x] Run targeted platform-risk tests inside the active app container.
|
|
|
|
Validation commands:
|
|
|
|
```bash
|
|
docker exec -w /app -e SERVER_VENV_DIR=/tmp/x-financial-server-venv x-financial-main /tmp/x-financial-server-venv/bin/pytest -q server/tests/test_expense_claim_platform_context_tools.py server/tests/test_expense_claim_platform_risk_stage.py
|
|
```
|
|
|
|
### Task 7: Frontend Protocol Constants And Test Helpers
|
|
|
|
**Files:**
|
|
|
|
- Create: `web/src/constants/documentProtocol.js`
|
|
- Create: `web/tests/helpers/sourceSurface.mjs`
|
|
- Create: `web/tests/document-protocol-constants.test.mjs`
|
|
- Modify: `web/src/composables/workbenchAiMode/workbenchAiApplicationPreviewModel.js`
|
|
- Migrate one high-churn source-surface test to the helper.
|
|
|
|
Steps:
|
|
|
|
- [x] Add failing Node tests for status labels, document type constants, and reusable source-surface loading.
|
|
- [x] Move duplicated status labels into `documentProtocol.js`.
|
|
- [x] Reuse constants in application-preview model and request/document-center model code.
|
|
- [x] Migrate one source-surface test helper to reduce brittle boilerplate.
|
|
|
|
Validation commands:
|
|
|
|
```bash
|
|
node --test web/tests/document-protocol-constants.test.mjs web/tests/workbench-ai-mode-switch.test.mjs
|
|
```
|
|
|
|
### Task 8: Backend Test Fixture Consolidation
|
|
|
|
**Files:**
|
|
|
|
- Create: `server/src/app/test_helpers/db.py`
|
|
- Create: `server/src/app/test_helpers/__init__.py`
|
|
- Create: `server/tests/test_db_test_helpers.py`
|
|
- Modify: `server/tests/test_expense_claim_platform_risk_stage.py`
|
|
|
|
Steps:
|
|
|
|
- [x] Identify an existing small test file with duplicated session/client/OCR helpers.
|
|
- [x] Add shared DB helper while preserving its current behavior.
|
|
- [x] Migrate one test file only.
|
|
- [x] Run the migrated test plus adjacent coverage inside the active app container.
|
|
|
|
Validation commands:
|
|
|
|
```bash
|
|
docker exec -w /app -e SERVER_VENV_DIR=/tmp/x-financial-server-venv x-financial-main /tmp/x-financial-server-venv/bin/pytest -q server/tests/test_db_test_helpers.py server/tests/test_expense_claim_platform_risk_stage.py
|
|
```
|
|
|
|
### Task 9: Steward Planner Module Split
|
|
|
|
**Files:**
|
|
|
|
- Create: `server/src/app/services/steward_planner_shared.py`
|
|
- Create: `server/src/app/services/steward_planner_fallback.py`
|
|
- Create: `server/src/app/services/steward_planner_extraction.py`
|
|
- Modify: `server/src/app/services/steward_planner.py`
|
|
|
|
Steps:
|
|
|
|
- [x] Move shared constants and `PlannedTaskDraft` into a shared planner module.
|
|
- [x] Move off-topic, pending-flow, and rule-fallback planning into `steward_planner_fallback.py`.
|
|
- [x] Move task extraction, ontology normalization, attachment grouping, and summary helpers into `steward_planner_extraction.py`.
|
|
- [x] Keep `steward_planner.py` as the service orchestration entrypoint.
|
|
- [x] Run planner/fact resolver tests inside the active app container.
|
|
|
|
Validation commands:
|
|
|
|
```bash
|
|
docker exec -w /app -e SERVER_VENV_DIR=/tmp/x-financial-server-venv x-financial-main /tmp/x-financial-server-venv/bin/pytest -q server/tests/test_application_fact_resolver.py server/tests/test_steward_planner.py
|
|
```
|
|
|
|
### Task 10: App Shell Dynamic Business Chunk Loading
|
|
|
|
**Files:**
|
|
|
|
- Create: `web/src/views/scripts/appShellAsyncViews.js`
|
|
- Create: `web/src/components/shared/AppViewLoadingState.vue`
|
|
- Create: `web/src/components/shared/AppModalLoadingState.vue`
|
|
- Modify: `web/src/views/AppShellRouteView.vue`
|
|
- Modify: `web/src/components/layout/SidebarRail.vue`
|
|
- Modify: `web/src/components/layout/AiSidebarRail.vue`
|
|
- Modify: `web/tests/app-shell-route-loading.test.mjs`
|
|
- Modify: `web/tests/ai-sidebar-rail-mode.test.mjs`
|
|
|
|
Steps:
|
|
|
|
- [x] Keep top-level shell routes eager so login/setup/app layout does not blank during route navigation.
|
|
- [x] Move heavy business views behind `defineAsyncComponent` loaders.
|
|
- [x] Add an in-workarea loading state for slow business chunks.
|
|
- [x] Add a modal loading state for the smart reimbursement assistant chunk.
|
|
- [x] Preload likely next views on sidebar hover/focus and during browser idle time.
|
|
- [x] Preserve existing Vite manual vendor chunks, including `vendor-echarts`.
|
|
- [x] Run targeted frontend tests and production build.
|
|
|
|
Validation commands:
|
|
|
|
```bash
|
|
node --test web/tests/app-shell-route-loading.test.mjs web/tests/ai-sidebar-rail-mode.test.mjs web/tests/sidebar-document-unread-dot.test.mjs web/tests/workbench-ai-mode-switch.test.mjs web/tests/workbench-ai-reimbursement-association-gate.test.mjs web/tests/travel-reimbursement-review-drawer-switch.test.mjs web/tests/documents-center-status-filter.test.mjs
|
|
npm --prefix web run build
|
|
```
|
|
|
|
Result:
|
|
|
|
- App shell entry chunk after split: `index-vWyUfHfm.js` 223.75 kB, gzip 74.11 kB.
|
|
- Large `vendor-echarts` chunk remains isolated at 598.67 kB, gzip 204.84 kB; it is no longer part of the app-shell entry chunk.
|
|
- Build still reports the existing Rollup `#__PURE__` annotation warnings from Element Plus / VueUse.
|
|
|
|
## Guardrails
|
|
|
|
- Do not touch unrelated dirty files.
|
|
- Do not change renderer output intentionally in this slice.
|
|
- Do not move backend logic until frontend helper extraction is green.
|
|
- Backend tests must run through Docker:
|
|
|
|
```bash
|
|
docker exec -w /app -e SERVER_VENV_DIR=/tmp/x-financial-server-venv x-financial-main /tmp/x-financial-server-venv/bin/pytest -q <path>
|
|
```
|