Files
Celes Renata b149f70507 fix: operator approval workflow — add approval toggle, lockout CRUD, and PBT tests
- Add GET/PUT /api/admin/trading/approval-config endpoints
- Add POST/DELETE /api/admin/trading/lockouts endpoints
- Add useApprovalConfig, useUpdateApprovalConfig, useCreateLockout, useDeleteLockout hooks
- Add Paper Order Approval toggle card with confirmation dialog
- Add lockout creation form and delete button to Active Lockouts card
- Add MSW handlers for all new endpoints
- Add property-based tests for bug condition exploration and preservation
2026-04-17 06:14:46 +00:00

183 lines
11 KiB
Markdown

# Implementation Plan
- [x] 1. Write bug condition exploration test
- **Property 1: Bug Condition** - Paper Mode Approval Bypass
- **CRITICAL**: This test MUST FAIL on unfixed code — failure confirms the bug exists
- **DO NOT attempt to fix the test or the code when it fails**
- **NOTE**: This test encodes the expected behavior — it will validate the fix when it passes after implementation
- **GOAL**: Surface counterexamples that demonstrate the bug exists
- **Scoped PBT Approach**: Scope the property to paper mode with `auto_approve_paper=False` — the bug is that even when `auto_approve_paper` is explicitly set to `False` in a `PortfolioRiskConfig`, the system works correctly at the function level, but the config JSON stored in `risk_configs` never contains `operator_approval` settings because no UI/endpoint sets them. The exploration test should:
1. Verify `requires_approval(config, PAPER)` returns `True` when `auto_approve_paper=False` (this actually passes — the function itself is correct)
2. Verify `PortfolioRiskConfig.from_db_json({})` produces `auto_approve_paper=True` (demonstrates the root cause — empty JSON always defaults to auto-approve)
3. Verify there is no dedicated endpoint to update `operator_approval` settings in the risk config (the existing `PUT /api/admin/trading/config` replaces the entire config JSON, and no UI calls it with `operator_approval` fields)
- Test file: `tests/test_pbt_operator_approval.py`
- Use Hypothesis with `@settings(max_examples=100)` per project conventions
- Generate random `PortfolioRiskConfig` instances with `auto_approve_paper=False` and `trading_mode=PAPER`
- Assert `requires_approval()` returns `True` for all such configs (function-level correctness)
- Also test that `PortfolioRiskConfig.from_db_json({})` always produces `auto_approve_paper=True` (demonstrates the config-level bug)
- Run test on UNFIXED code
- **EXPECTED OUTCOME**: The function-level property passes (function is correct), but the `from_db_json({})` test demonstrates the root cause — empty config always auto-approves
- Document counterexamples found to understand root cause
- Mark task complete when test is written, run, and results are documented
- _Requirements: 1.1, 1.2, 1.5, 2.1_
- [x] 2. Write preservation property tests (BEFORE implementing fix)
- **Property 2: Preservation** - Non-Paper-Toggle Approval Behavior
- **IMPORTANT**: Follow observation-first methodology
- Observe behavior on UNFIXED code for non-buggy inputs:
- Observe: `requires_approval(config, LIVE)` returns `True` when `require_approval_for_live=True`
- Observe: `requires_approval(config, LIVE)` returns `False` when `require_approval_for_live=False`
- Observe: `requires_approval(config, DISABLED)` returns `False` for all configs
- Observe: `requires_approval(config, PAPER)` returns `False` when `auto_approve_paper=True`
- Write property-based tests capturing observed behavior patterns:
- For all `(trading_mode, auto_approve_paper, require_approval_for_live)` combinations where `trading_mode != PAPER` OR `auto_approve_paper == True`, verify `requires_approval()` returns the same result as the original function
- Property: for all LIVE mode configs, result equals `require_approval_for_live`
- Property: for all DISABLED mode configs, result is always `False`
- Property: for all PAPER mode configs with `auto_approve_paper=True`, result is always `False`
- Test file: `tests/test_pbt_operator_approval.py` (same file, separate test class)
- Use Hypothesis with `@settings(max_examples=100)` per project conventions
- Verify tests PASS on UNFIXED code
- **EXPECTED OUTCOME**: Tests PASS (confirms baseline behavior to preserve)
- Mark task complete when tests are written, run, and passing on unfixed code
- _Requirements: 3.1, 3.2, 3.3, 3.4, 3.5, 3.6_
- [x] 3. Implement backend API endpoints
- [x] 3.1 Add `GET /api/admin/trading/approval-config` endpoint
- Read the active `risk_configs` row and extract `operator_approval` settings from the `config` JSONB column
- Parse with `PortfolioRiskConfig.from_db_json()` to get defaults when fields are missing
- Return `{ auto_approve_paper: bool, require_approval_for_live: bool, approval_timeout_minutes: int }`
- File: `services/api/app.py`
- _Bug_Condition: isBugCondition(input) where risk_config_json has no "operator_approval" key_
- _Expected_Behavior: Endpoint returns current operator_approval settings (with defaults filled in)_
- _Preservation: Existing GET /api/admin/trading/config endpoint unchanged_
- _Requirements: 1.5, 2.5_
- [x] 3.2 Add `PUT /api/admin/trading/approval-config` endpoint
- Read current `risk_configs.config` JSONB, merge `operator_approval` sub-object, write back
- Accept `{ auto_approve_paper: bool }` (and optionally `require_approval_for_live`, `approval_timeout_minutes`)
- Use JSON merge (not full replace) to preserve other config fields
- File: `services/api/app.py`
- _Bug_Condition: No UI/endpoint to set auto_approve_paper=False in risk_config_json_
- _Expected_Behavior: PUT updates operator_approval in config JSONB, requires_approval() reflects the change_
- _Preservation: Other config fields (position_limits, sector_exposure, etc.) unchanged by merge_
- _Requirements: 1.5, 2.1, 2.5_
- [x] 3.3 Add `POST /api/admin/trading/lockouts` endpoint
- Accept `{ ticker: string, reason: string, duration_minutes: int, lockout_type?: string }`
- Compute `expires_at = NOW() + duration_minutes`
- Default `lockout_type` to `"manual"`
- Insert into `symbol_lockouts` table
- Return the created lockout row
- File: `services/api/app.py`
- _Bug_Condition: No POST endpoint exists for symbol_lockouts_
- _Expected_Behavior: Manual lockout created and visible in GET /api/admin/trading/lockouts_
- _Preservation: System-generated lockouts (news-shock, cooldown) unaffected_
- _Requirements: 1.3, 1.4, 2.3, 2.4, 3.6_
- [x] 3.4 Add `DELETE /api/admin/trading/lockouts/{lockout_id}` endpoint
- Delete the lockout row by ID
- Return 404 if not found
- File: `services/api/app.py`
- _Bug_Condition: No DELETE endpoint exists for symbol_lockouts_
- _Expected_Behavior: Lockout removed early, no longer appears in GET results_
- _Preservation: Other lockouts unaffected_
- _Requirements: 1.4, 2.4_
- [x] 4. Implement frontend hooks
- [x] 4.1 Add `useApprovalConfig()` hook
- Fetches `GET /api/admin/trading/approval-config` via `useGet` with query key `['approval-config']`
- Type the response as `{ auto_approve_paper: boolean, require_approval_for_live: boolean, approval_timeout_minutes: number }`
- File: `frontend/src/api/hooks.ts`
- _Requirements: 2.5_
- [x] 4.2 Add `useUpdateApprovalConfig()` mutation hook
- Calls `PUT /api/admin/trading/approval-config` via `apiPut`
- Invalidates `['approval-config']` and `['trading-config']` query keys on success
- File: `frontend/src/api/hooks.ts`
- _Requirements: 2.1, 2.5_
- [x] 4.3 Add `useCreateLockout()` mutation hook
- Calls `POST /api/admin/trading/lockouts` via `apiPost`
- Accepts `{ ticker: string, reason: string, duration_minutes: number }`
- Invalidates `['lockouts']` query key on success
- File: `frontend/src/api/hooks.ts`
- _Requirements: 2.3_
- [x] 4.4 Add `useDeleteLockout()` mutation hook
- Calls `DELETE /api/admin/trading/lockouts/{id}` via `apiDelete`
- Invalidates `['lockouts']` query key on success
- File: `frontend/src/api/hooks.ts`
- _Requirements: 2.4_
- [x] 5. Implement frontend UI controls
- [x] 5.1 Add Approval Toggle Card to Trading page
- New `Card` between the existing signal layer toggles and Pending Approvals section
- Toggle switch for `auto_approve_paper` using `useApprovalConfig()` and `useUpdateApprovalConfig()`
- Confirmation dialog before toggling (since enabling approval requirements changes order flow)
- Show current state label ("Auto-approve paper orders" / "Require approval for paper orders")
- Use same toggle switch pattern as Macro Signal Layer and Competitive Signal Layer cards
- File: `frontend/src/pages/Trading.tsx`
- _Requirements: 1.5, 2.1, 2.5_
- [x] 5.2 Add Lockout Creation Form to Active Lockouts card
- Inline form at the top of the Active Lockouts card with fields:
- Ticker (text input, uppercase)
- Reason (text input)
- Duration in minutes (number input, default 60)
- Submit button calls `useCreateLockout()`
- Clear form on successful submission
- File: `frontend/src/pages/Trading.tsx`
- _Requirements: 1.3, 1.4, 2.3, 2.4_
- [x] 5.3 Add Lockout Delete Button to each lockout row
- "Remove" button on each lockout entry in the Active Lockouts list
- Calls `useDeleteLockout()` with the lockout ID
- File: `frontend/src/pages/Trading.tsx`
- _Requirements: 1.4, 2.4_
- [x] 5.4 Add MSW handlers for new endpoints
- Add mock handlers in `frontend/src/test/mocks/handlers.ts` for:
- `GET /api/admin/trading/approval-config`
- `PUT /api/admin/trading/approval-config`
- `POST /api/admin/trading/lockouts`
- `DELETE /api/admin/trading/lockouts/:id`
- File: `frontend/src/test/mocks/handlers.ts`
- _Requirements: 2.1, 2.3, 2.4, 2.5_
- [x] 6. Verify bug condition exploration test now passes
- [x] 6.1 Re-run bug condition exploration test
- **Property 1: Expected Behavior** - Paper Mode Approval Toggle
- **IMPORTANT**: Re-run the SAME test from task 1 — do NOT write a new test
- The test from task 1 encodes the expected behavior
- With the new `PUT /api/admin/trading/approval-config` endpoint in place, the operator can now set `auto_approve_paper=False` in the risk config JSON
- The `requires_approval()` function-level property should still pass
- The config-level test should now demonstrate that the endpoint correctly updates the config
- Run: `.venv/bin/python -m pytest tests/test_pbt_operator_approval.py -x --tb=short -q`
- **EXPECTED OUTCOME**: Test PASSES (confirms bug is fixed — operator can now control approval settings)
- _Requirements: 2.1, 2.2, 2.5_
- [x] 6.2 Verify preservation tests still pass
- **Property 2: Preservation** - Non-Paper-Toggle Approval Behavior
- **IMPORTANT**: Re-run the SAME tests from task 2 — do NOT write new tests
- Run: `.venv/bin/python -m pytest tests/test_pbt_operator_approval.py -x --tb=short -q`
- **EXPECTED OUTCOME**: Tests PASS (confirms no regressions)
- Confirm all preservation properties still hold after the fix
- _Requirements: 3.1, 3.2, 3.3, 3.4, 3.5, 3.6_
- [x] 7. Run frontend tests
- Run: `cd frontend && npx vitest --run`
- Verify Trading page renders correctly with new controls
- Verify no regressions in existing frontend tests
- _Requirements: 2.1, 2.3, 2.4, 2.5_
- [x] 8. Checkpoint — Ensure all tests pass
- Run full Python test suite: `.venv/bin/python -m pytest tests/test_pbt_operator_approval.py -x --tb=short -q`
- Run full frontend test suite: `cd frontend && npx vitest --run`
- Ensure all property-based tests pass (both bug condition and preservation)
- Ensure all frontend tests pass (including new MSW handlers)
- Ask the user if questions arise