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

11 KiB

Implementation Plan

  • 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
  • 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
  • 3. Implement backend API endpoints

    • 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
    • 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
    • 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
    • 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
  • 4. Implement frontend hooks

    • 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
    • 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
    • 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
    • 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
  • 5. Implement frontend UI controls

    • 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
    • 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
    • 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
    • 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
  • 6. Verify bug condition exploration test now passes

    • 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
    • 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
  • 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
  • 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