Review: Late-Game Lock: entry logic + OrderService integration

review-1749-2026-07-04 Review

review needs-refinement

Verdict: NEEDS_REFINEMENT

Scope is solid and implementation-ready. Single issue: missing arch-rails backing note in pal-e-docs (systemic gap across all arch:rails board items).

Template Completeness

  • [x] Type (Feature)
  • [x] Lineage
  • [x] Repo
  • [x] User Story
  • [x] Context
  • [x] File Targets
  • [x] Feature Flag
  • [x] Acceptance Criteria
  • [x] Test Expectations
  • [x] Constraints
  • [x] Checklist
  • [x] Related

Traceability

  • [x] story:watchdog-trading label -- Watchdog Trading story found in project-prediction-assistant user-stories section
  • [x] story note verified -- entry exists on project page with key, role, and success metric
  • [x] arch:rails label present
  • [ ] arch note MISSING -- [SCOPE] Create architecture note arch-rails for the Rails component. This is a systemic gap affecting all arch:rails items on the board.
  • [x] Forgejo issue -- ldraney/prediction-assistant#68, open

File Targets

  • [x] app/services/late_game_lock_bot.rb -- verified: does not exist yet, to be created (new bot service)
  • [x] app/services/late_game_lock_evaluator.rb -- verified: does not exist yet. Created by dependency #66 (event dispatch sub-ticket). Ticket correctly notes dependency in Lineage section: "Depends on the event dispatch sub-ticket (evaluator must exist to call the bot)."
  • [x] test/services/late_game_lock_bot_test.rb -- verified: does not exist yet, to be created
  • [x] app/services/order_service.rb (NOT to touch) -- verified exists. place_order, DuplicateOrderError, PermanentApiError, dry_run all confirmed present.
  • [x] app/services/position_tracker.rb (NOT to touch) -- verified exists. open_positions(bot:) and conflict?(ticker:, side:, exclude_strategy:) signatures confirmed.
  • [x] app/models/late_game_lock_config.rb (NOT to touch) -- verified exists. Inherits from Strategy (which has has_many :trades). All referenced config keys present: bet_size, max_concurrent_positions, price_threshold, stop_loss, take_profit, auto_sell_vs_hold.

Repo Placement

OK. Issue filed on ldraney/prediction-assistant, all file targets are in the same repo.

Dependencies

  • #66 (event dispatch wiring + dual-trigger evaluator) -- BLOCKING dependency. Creates late_game_lock_evaluator.rb that this ticket modifies. Currently in backlog (item #1747). Must complete before this ticket can start. Documented in Lineage section.
  • #23 (OrderService) -- merged (in validation column). Provides place_order, error classes, dry-run support. Verified on disk.
  • #24 (PositionTracker) -- merged (in validation column). Provides open_positions, conflict?. Verified on disk.
  • #70 (exit management, 3rd sub-ticket) -- depends on THIS ticket. Will extend late_game_lock_bot.rb with process_exit method.
  • #4 (parent issue) -- in todo column. Decomposed into #66 -> #68 -> #70.
Dependency chain is well-documented and correctly ordered.

Acceptance Criteria

5 ACs, all agent-verifiable:
  • AC1 (YES buy order): testable with mocked OrderService
  • AC2 (max_concurrent_positions): testable with PositionTracker mock returning varying counts
  • AC3 (dedup guard): testable -- exact pattern from PregameStackerBot#already_trading? confirmed: config.trades.where(ticker: ticker, status: %w[pending filled]).exists?
  • AC4 (conflict detection): testable with PositionTracker.conflict? mock
  • AC5 (logging): testable by asserting log output
Test command provided: rails test test/services/late_game_lock_bot_test.rb
Minor note: AC1 references config.bet_size but actual accessor pattern in codebase is config.config["bet_size"] (JSON column). Agent should follow PregameStackerBot pattern. Not blocking -- intent is clear.

Blast Radius

Low risk. This ticket creates new files (late_game_lock_bot.rb, tests) and modifies one file created by a dependency (late_game_lock_evaluator.rb). No changes to shared services. Follows identical patterns to PregameStackerBot -- same OrderService API, same PositionTracker API, same dedup logic. No downstream consumers affected beyond #70 which extends the same file.

Decomposition Assessment

  • 3 file targets (2 new, 1 modify) across 1 repo -- within limit
  • 5 acceptance criteria -- at boundary, acceptable
  • Estimated agent work: ~3-4 minutes (follows well-established PregameStackerBot pattern)
No decomposition needed.

Recommendation

  • [SCOPE] Create architecture note arch-rails for the Rails component in pal-e-docs. This is a systemic gap -- no arch-rails note exists, and the label is used across many board items. This is not a blocker for implementation but blocks full traceability compliance.