feat: persist review workbench decisions (#1)
Merge dcy-012 / REQ-020 / BAT-016 implementation after green CI.
This commit is contained in:
@@ -1,31 +1,47 @@
|
||||
# BAT-016 Design: Review decision persistence and reconciliation workbench
|
||||
|
||||
## Design Status
|
||||
- Status: draft
|
||||
- Owner: delivery-runner to complete with dev-impl input before implementation.
|
||||
- Status: completed
|
||||
- Owner: 가웨인 direct recovery path; Kanban prohibited by user.
|
||||
|
||||
## Required Design Decisions
|
||||
1. Persistence schema
|
||||
- Define exact columns, indexes, uniqueness/idempotency keys, and schema migration path for `review_decisions` and `reconciliation_sessions`.
|
||||
- Define whether target payload is JSON text and how sensitive content is minimized.
|
||||
2. State model
|
||||
- Map UI labels to internal `status` and `decision_type` values from REQ-020.
|
||||
- Define allowed transitions for pending/decided/held/needs_recheck/ignored.
|
||||
3. Application contracts
|
||||
- Define additive fields returned by review queue, reconciliation report, and uncategorized workbench services.
|
||||
- Define compatibility expectations for existing MCP and Streamlit consumers.
|
||||
4. MCP tool contracts
|
||||
- Define names, arguments, responses, error handling, and non-goals for review decision/session tools.
|
||||
- Explicitly exclude destructive delete/reset operations.
|
||||
5. Streamlit interaction model
|
||||
- Define minimum UI actions, confirmation patterns, and feedback messages.
|
||||
6. Verification design
|
||||
- Define fixtures and representative scenarios for persistence round-trip, contract compatibility, dashboard behavior, and DB safety.
|
||||
## Architecture Summary
|
||||
BAT-016 adds a small review-state layer beside the existing ledger facts. Transactions, accounts, imports, categories, and reports remain the financial source of truth; `review_decisions` and `reconciliation_sessions` store operator workflow state only. MCP and Streamlit adapters use the existing `service.py` facade and page/domain gateways, so the new behavior is additive to current consumers.
|
||||
|
||||
## Initial Recommendation
|
||||
Use a common decision envelope with fields aligned to REQ-020 and target-specific payload fields stored as minimal JSON. Use reconciliation sessions as account/basis-date close records and allow review decisions to target either a session or a specific item.
|
||||
## Changed Areas
|
||||
- SQLite schema/migrations: schema version `11`, new `review_decisions` and `reconciliation_sessions` tables plus indexes.
|
||||
- Application: new `review_decision_service.py` owns decision/session persistence, payload sanitization, operating indicators, and attachment helpers.
|
||||
- Existing services: cross-transfer queue, reconciliation report, and uncategorized workbench attach persisted review/session state with additive fields.
|
||||
- MCP: safe review decision/session tools registered through `tool_contracts.py` and `server.py`.
|
||||
- Streamlit: dashboard actions for review decisions and reconciliation sessions through existing gateways.
|
||||
- Docs/tests: data model/project structure/runtime flow baselines and pytest coverage.
|
||||
|
||||
## Design Completion Criteria
|
||||
- Schema and state transitions are explicit enough for dev-impl to implement without product reinterpretation.
|
||||
- MCP and Streamlit contracts are additive and testable.
|
||||
- Verification scenarios map directly to REQ-020 AC 1-10.
|
||||
## Key Decisions
|
||||
1. **Schema**
|
||||
- `review_decisions`: common decision envelope with `target_type`, `target_id`, `status`, `decision_type`, `reason_code`, `memo`, `operator`, `decided_at`, `review_after`, `target_payload`, timestamps.
|
||||
- `reconciliation_sessions`: account/basis-date session records with expected/actual/difference, `status`, `closed_at`, source/memo/operator, timestamps.
|
||||
- Migrations are additive and idempotent using `CREATE TABLE IF NOT EXISTS` and `CREATE INDEX IF NOT EXISTS`.
|
||||
2. **State model**
|
||||
- Review statuses: `pending`, `decided`, `held`, `needs_recheck`, `ignored`.
|
||||
- Decision types: `account_transfer`, `real_expense`, `real_income`, `investment_transfer`, `excluded_from_analysis`, `duplicate_or_cancel`, `unknown`.
|
||||
- Reconciliation statuses: `open`, `closed`, `needs_recheck`, `ignored`.
|
||||
3. **Payload safety**
|
||||
- `target_payload` is minimal JSON text; known sensitive keys such as `raw_payload`, password/token/secret/file_path are dropped before persistence.
|
||||
4. **Contracts**
|
||||
- Existing payloads remain compatible; new fields such as `review_status`, `review_decision`, and `reconciliation_session` are additive.
|
||||
- MCP adds non-destructive tools only; no delete/reset tool is exposed.
|
||||
5. **Operating indicators**
|
||||
- The service computes pending review, held/recheck, unreconciled account count, last reconciliation close time, and uncategorized recommendation backlog from persisted state plus current ledger state.
|
||||
|
||||
## Edge Cases
|
||||
- Unknown/invalid status or decision type returns validation errors rather than writing inconsistent state.
|
||||
- Release/hold/recheck operations target the latest decision for a given target and preserve audit timestamps.
|
||||
- Reconciliation session `difference` can be supplied or derived from expected/actual values.
|
||||
- Empty or unparsable `target_payload` is reduced to `{}` or a short summary, never raw financial evidence expansion.
|
||||
|
||||
## Architecture Impact
|
||||
- `both`: project structure gains `application/review_decision_service.py`; runtime flows gain review-decision/reconciliation workbench flow.
|
||||
|
||||
## Reference Doc Update Plan
|
||||
- `docs/data-model.md`: add review/session entities, state values, and payload-minimization rules.
|
||||
- `docs/project-structure.md`: list `review_decision_service.py` ownership.
|
||||
- `docs/runtime-flows.md`: add review decision and reconciliation workbench runtime flow.
|
||||
|
||||
+45
-10
@@ -1,17 +1,52 @@
|
||||
# BAT-016 Implementation Log
|
||||
|
||||
## Status
|
||||
- Not started.
|
||||
- Implemented locally; release-candidate after verification.
|
||||
- Completed At (KST): 2026-07-08 07:11 KST
|
||||
- Branch: `feat/bat-016-review-decision-workbench`
|
||||
|
||||
## Implementation Owner
|
||||
- dev-builder / dev-impl under delivery-runner orchestration.
|
||||
- 가웨인 direct recovery path. User explicitly prohibited Kanban after prior worker failures.
|
||||
|
||||
## Expected Evidence
|
||||
- Code diff summary by slice.
|
||||
- Migration and validation DB evidence.
|
||||
- MCP tool discovery and representative call evidence.
|
||||
- Streamlit AppTest/HTTP smoke evidence.
|
||||
- Full lint/test evidence.
|
||||
## Changed Files
|
||||
- `jarca_ledger_mcp/infrastructure/sqlite/schema.py`
|
||||
- `jarca_ledger_mcp/infrastructure/sqlite/migrations.py`
|
||||
- `jarca_ledger_mcp/application/review_decision_service.py`
|
||||
- `jarca_ledger_mcp/application/cross_transfer_service.py`
|
||||
- `jarca_ledger_mcp/application/reconciliation_service.py`
|
||||
- `jarca_ledger_mcp/application/uncategorized_workbench_service.py`
|
||||
- `jarca_ledger_mcp/application/system_services.py`
|
||||
- `jarca_ledger_mcp/service.py`
|
||||
- `jarca_ledger_mcp/adapters/mcp/tool_contracts.py`
|
||||
- `jarca_ledger_mcp/adapters/mcp/server.py`
|
||||
- `jarca_ledger_mcp/adapters/streamlit/transfers_gateway.py`
|
||||
- `jarca_ledger_mcp/adapters/streamlit/reconciliation_gateway.py`
|
||||
- `jarca_ledger_mcp/adapters/streamlit/categorization_gateway.py`
|
||||
- `dashboard/app.py`
|
||||
- `tests/test_review_decision_workbench.py`
|
||||
- `tests/test_service.py`
|
||||
- `tests/test_ledger_categories.py`
|
||||
- `docs/data-model.md`
|
||||
- `docs/project-structure.md`
|
||||
- `docs/runtime-flows.md`
|
||||
|
||||
## Log
|
||||
- 2026-07-08 00:00 KST: Batch scaffolded by lead. No implementation performed yet.
|
||||
## Implementation Summary
|
||||
- Raised SQLite schema version to `11` and added additive/idempotent `review_decisions` and `reconciliation_sessions` schema objects.
|
||||
- Added `review_decision_service.py` for safe non-destructive review decision and reconciliation session operations.
|
||||
- Added payload minimization to prevent raw payload/password/token/secret/file-path persistence.
|
||||
- Added review/session state enrichment to cross-transfer queue, balance reconciliation report, and uncategorized workbench outputs.
|
||||
- Added MCP tools for list/upsert/hold/recheck/release review decisions, list/upsert/close reconciliation sessions, and review operating indicators.
|
||||
- Added Streamlit actions and gateway exports for review decision/session workflow state.
|
||||
- Added regression tests for schema idempotency, sensitive payload filtering, reconciliation session round trip, report integration, and MCP tool surface.
|
||||
|
||||
## Commands and Results
|
||||
- `python -m ruff check .` → passed.
|
||||
- `python -m pytest tests/test_review_decision_workbench.py -q` → `.... [100%]`.
|
||||
- `python -m pytest -q` → passed, with existing skipped tests preserved.
|
||||
- MCP surface smoke: required 9 review/session tools present; destructive delete/reset absent; tool count `65`.
|
||||
- DB safety smoke: schema version `11`, `PRAGMA integrity_check` `ok`, new tables present and empty on synthetic DB.
|
||||
|
||||
## Notes
|
||||
- No production DB destructive operation was performed.
|
||||
- No raw financial evidence or secrets were added to repository fixtures.
|
||||
- Final PR/CI evidence remains the next gate after commit/push.
|
||||
|
||||
@@ -1,16 +1,17 @@
|
||||
# BAT-016: Review decision persistence and reconciliation workbench
|
||||
|
||||
- Profile: standard
|
||||
- Status: draft
|
||||
- Status: release-candidate
|
||||
- Included REQ: REQ-020
|
||||
- Source Discovery: dcy-012
|
||||
- Created At (KST): 2026-07-08 00:00 KST
|
||||
- Verification Completed At (KST): 2026-07-08 07:11 KST
|
||||
|
||||
## Objective
|
||||
Deliver the approved REQ-020 scope for persisted review decisions, reconciliation sessions, Streamlit workbench actions, network MCP additive tools, operating hub indicators, and baseline documentation.
|
||||
|
||||
## Included Scope
|
||||
- Additive/idempotent persistence for `review_decisions` and `reconciliation_sessions` or an equivalent structure.
|
||||
- Additive/idempotent persistence for `review_decisions` and `reconciliation_sessions`.
|
||||
- Application/service integration for cross-transfer review queue, reconciliation report, and uncategorized workbench.
|
||||
- Streamlit workbench actions for decision create/update/hold/recheck/release.
|
||||
- Network MCP tools for safe review decision/session read and mutation operations.
|
||||
@@ -31,6 +32,6 @@ Deliver the approved REQ-020 scope for persisted review decisions, reconciliatio
|
||||
- [implementation.md](implementation.md)
|
||||
- [verification.md](verification.md)
|
||||
|
||||
## Harness Notes
|
||||
- This batch is `standard` because REQ-020 changes persistence, interface surface, runtime flow, Streamlit UI, MCP tools, and baseline docs.
|
||||
- After this batch scaffold is committed, lead should issue a kanban-backed `lead -> delivery-runner` kickoff on the shared development execution board.
|
||||
## Delivery Notes
|
||||
- User explicitly prohibited Kanban for this recovery path; delivery was completed directly on feature branch `feat/bat-016-review-decision-workbench`.
|
||||
- Local verification evidence is recorded in `verification.md`; final merge remains subject to PR/CI review.
|
||||
|
||||
@@ -1,18 +1,18 @@
|
||||
# BAT-016 Planning: Review decision persistence and reconciliation workbench
|
||||
|
||||
## Planning Status
|
||||
- Status: draft
|
||||
- Status: completed
|
||||
- Profile: standard
|
||||
- Design Gate: yes
|
||||
- Design Gate Rationale: REQ-020 changes persistence schema, application service contracts, MCP tools, Streamlit UI flow, and operating hub aggregation. A design document is required before implementation.
|
||||
|
||||
## Objective
|
||||
Turn REQ-020 into a delivery plan that can be executed by delivery-runner with separated implementation and QC roles.
|
||||
Turn REQ-020 into a delivery plan for persisted review decisions and reconciliation sessions without changing existing public contracts destructively.
|
||||
|
||||
## Work Slices
|
||||
1. Persistence foundation
|
||||
- Add review decision and reconciliation session schema via additive/idempotent migration.
|
||||
- Add repository/service helpers with validation DB coverage.
|
||||
- Add service helpers with validation DB coverage.
|
||||
2. Application integration
|
||||
- Connect cross-transfer review queue, reconciliation report, and uncategorized workbench to persisted decision/session state.
|
||||
- Preserve existing response fields with additive extensions only.
|
||||
@@ -24,11 +24,22 @@ Turn REQ-020 into a delivery plan that can be executed by delivery-runner with s
|
||||
- Exclude destructive delete/reset tools.
|
||||
5. Baseline and verification
|
||||
- Update data model, project structure, runtime flows.
|
||||
- Add pytest, contract/smoke, Streamlit AppTest/HTTP smoke, and DB safety evidence.
|
||||
- Add pytest, MCP surface smoke, Streamlit/dashboard and DB safety evidence.
|
||||
|
||||
## Acceptance Source
|
||||
- REQ-020 Acceptance Criteria 1-10.
|
||||
|
||||
## Dependencies
|
||||
- dcy-013/BAT-015 network MCP endpoint remains the deployment substrate; BAT-016 does not redesign transport.
|
||||
- Existing SQLite migration path must remain additive/idempotent.
|
||||
- Existing dashboard and MCP consumers must keep their current response fields.
|
||||
|
||||
## Milestones
|
||||
1. Schema + service implementation.
|
||||
2. Adapter/UI integration.
|
||||
3. Targeted tests and full local regression.
|
||||
4. StagePilot artifact update and PR/CI validation.
|
||||
|
||||
## Risks
|
||||
- Source-of-truth ambiguity between current transaction/account state and persisted decision state.
|
||||
- Session granularity mismatch across account-level reconciliation and item-level review decisions.
|
||||
@@ -36,6 +47,7 @@ Turn REQ-020 into a delivery plan that can be executed by delivery-runner with s
|
||||
- MCP/Streamlit contract drift.
|
||||
|
||||
## Handoff Readiness
|
||||
- Ready for design drafting: yes.
|
||||
- Ready for implementation: no; design must define schema, state transitions, and interface contracts first.
|
||||
- Ready for design drafting: completed.
|
||||
- Ready for implementation: completed after design decisions below.
|
||||
- Lead authority needed before delivery: no additional user decision is currently open; scope is approved in dcy-012 and REQ-020.
|
||||
- Kanban status: deliberately not used for this recovery path per explicit user instruction.
|
||||
|
||||
+66
-14
@@ -1,22 +1,74 @@
|
||||
# BAT-016 Verification
|
||||
|
||||
## Status
|
||||
- Not started.
|
||||
- local-pass
|
||||
- Verified At (KST): 2026-07-08 07:11 KST
|
||||
|
||||
## QC Owner
|
||||
- dev-reviewer / dev-qc under delivery-runner orchestration.
|
||||
- 가웨인 direct recovery path; independent QC still recommended before merge because Kanban-based dev-reviewer was explicitly disabled by user instruction.
|
||||
|
||||
## Required Verification
|
||||
- REQ-020 AC 1-10 must be verified with direct evidence.
|
||||
- `ruff check .` must pass.
|
||||
- Full pytest must pass.
|
||||
- MCP contract/regression and network MCP representative smoke must pass.
|
||||
- Streamlit AppTest and HTTP smoke must pass.
|
||||
- DB safety/migration regression must pass.
|
||||
- Baseline document updates must be inspected.
|
||||
## Required Verification Mapping
|
||||
| REQ-020 AC | Evidence |
|
||||
|---|---|
|
||||
| AC1 additive/idempotent persistence | `tests/test_review_decision_workbench.py::test_review_decision_and_reconciliation_schema_are_additive_idempotent`; DB safety smoke showed schema `11`, integrity `ok`. |
|
||||
| AC2 review decision round-trip | `test_review_decision_round_trip_payload_sanitizes_sensitive_fields`. |
|
||||
| AC3 reconciliation session round-trip/hub basis | `test_reconciliation_session_round_trip_and_report_integration`. |
|
||||
| AC4 additive existing service payloads | Full pytest passed; report/workbench tests preserved; new fields are additive. |
|
||||
| AC5 network MCP safe tools | MCP tool surface smoke found required review/session tools and no destructive delete/reset tools. |
|
||||
| AC6 Streamlit workbench regression | Full pytest includes `tests/test_dashboard_app.py`; local pass. |
|
||||
| AC7 operating indicators | `get_review_operating_indicators` included in MCP smoke and session test checks last reconciliation close time. |
|
||||
| AC8 financial data safety | Payload sanitization test drops `raw_payload` and `file_path`; no real DB or raw evidence used. |
|
||||
| AC9 regression gates | `ruff check .` and full pytest passed locally. CI/PR evidence pending. |
|
||||
| AC10 baseline docs | `docs/data-model.md`, `docs/project-structure.md`, `docs/runtime-flows.md` updated. |
|
||||
|
||||
## Local Evidence
|
||||
```text
|
||||
python -m ruff check .
|
||||
All checks passed!
|
||||
|
||||
python -m pytest tests/test_review_decision_workbench.py -q
|
||||
.... [100%]
|
||||
|
||||
python -m pytest -q
|
||||
........................ss......................ss... [100%]
|
||||
|
||||
python -m pytest tests/test_contract_regression.py -q
|
||||
... [100%]
|
||||
|
||||
python -m pytest --ignore=tests/test_dashboard_app.py -q
|
||||
....................ss......................ss... [100%]
|
||||
|
||||
python -m pytest tests/test_dashboard_app.py -q
|
||||
.... [100%]
|
||||
|
||||
python scripts/ci_dashboard_http_smoke.py --host 127.0.0.1 --port 18501 --timeout 45
|
||||
success=true, http_status=200, fixture=synthetic
|
||||
|
||||
python scripts/ci_db_safety_smoke.py --timeout 45
|
||||
success=true, fixture_integrity=ok, restored_integrity=ok, operating_db_used=false
|
||||
|
||||
MCP surface smoke
|
||||
close_reconciliation_session OK
|
||||
get_review_operating_indicators OK
|
||||
hold_review_decision OK
|
||||
list_reconciliation_sessions OK
|
||||
list_review_decisions OK
|
||||
mark_review_recheck OK
|
||||
release_review_decision OK
|
||||
upsert_reconciliation_session OK
|
||||
upsert_review_decision OK
|
||||
tool_count 65
|
||||
|
||||
DB safety smoke
|
||||
schema_version 11
|
||||
integrity_check ok
|
||||
review_decisions 0
|
||||
reconciliation_sessions 0
|
||||
```
|
||||
|
||||
## Residual Risk
|
||||
- Independent dev-reviewer QC was not run because user prohibited Kanban. Treat PR review/CI as the next independent gate.
|
||||
- Local verification passed, but GitHub Actions CI must still be checked after push.
|
||||
|
||||
## Verdict
|
||||
- Pending.
|
||||
|
||||
## Evidence
|
||||
- None yet.
|
||||
- Local release-candidate.
|
||||
|
||||
@@ -23,4 +23,6 @@
|
||||
| BAT-013 | standard | release-candidate | REQ-017 | dcy-011 | [bat-013_20260703_advanced-workflow-service-split](bat-013_20260703_advanced-workflow-service-split/) |
|
||||
| BAT-014 | standard | release-candidate | REQ-018 | dcy-011 | [bat-014_20260703_adapter-surface-narrowing-baseline-refresh](bat-014_20260703_adapter-surface-narrowing-baseline-refresh/) |
|
||||
| BAT-015 | standard | release-candidate | REQ-019 | dcy-013 | [bat-015_20260707_docker-network-mcp-deployment](bat-015_20260707_docker-network-mcp-deployment/) |
|
||||
| BAT-016 | standard | draft | REQ-020 | dcy-012 | [bat-016_20260708_review-decision-reconciliation-workbench](bat-016_20260708_review-decision-reconciliation-workbench/) |
|
||||
| BAT-016 | standard | release-candidate | REQ-020 | dcy-012 | [bat-016_20260708_review-decision-reconciliation-workbench](bat-016_20260708_review-decision-reconciliation-workbench/) |
|
||||
|
||||
- 2026-07-08 07:11 KST: BAT-016을 local verification 통과 근거로 release-candidate 상태로 전환했다. Kanban은 사용자 명시 지시로 사용하지 않았다.
|
||||
|
||||
Reference in New Issue
Block a user