Review: EdgeLearnerJob consumer + fan-out wiring

review-1752-2026-07-04 Review

review ready

Verdict: READY

Board item #1752 -- Forgejo issue ldraney/prediction-assistant#71. Re-review after issue update (issue updated 12:05 UTC, prior review at 10:32 UTC). Prior [BODY] findings largely resolved. All findings resolved. Prior arch-rails gap was a false positive (note exists at slug arch-rails, confirmed by multiple agents).

Template Completeness

  • [x] Type -- Feature
  • [x] Lineage -- Sprint 6, sub-ticket of #29
  • [x] Repo -- ldraney/prediction-assistant
  • [x] User Story -- "AI Portfolio Builder" (link reference format; matches story:portfolio-builder label). Prior review flagged "Bot Marketplace" mismatch -- now FIXED in current issue body.
  • [x] Context -- thorough; explains 3 event types, delegation targets, fan-out wiring needs with specific code references
  • [x] File Targets -- 5 targets (3 modify, 2 new)
  • [x] Feature Flag -- None (activation via EdgeLearnerConfig active toggle)
  • [x] Dependencies -- documented with issue refs and explicit ordering
  • [x] Merge Coordination -- clear merge order: PricePattern -> GroupBuilder -> EdgeLearnerJob, plus Late-Game Lock coordination
  • [x] Acceptance Criteria -- 5 items
  • [x] Test Expectations -- 5 specific test scenarios
  • [x] Constraints -- 4 constraints including concurrency and learning feedback
  • [x] Checklist -- standard 3-item
  • [x] Related -- parent and dependency references

Traceability

  • [x] story:portfolio-builder label -- present on board item
  • [x] story note verified -- "portfolio-builder" found in project-prediction-assistant user-stories section (row: Trader (Lucas), links to story-prediction-assistant-portfolio-builder)
  • [x] arch:rails label -- present on board item
  • [x] arch note verified -- arch-rails note exists in pal-e-docs (confirmed by multiple agents; prior "missing" finding was a false positive)
  • [x] Forgejo issue -- #71, open, valid URL

File Targets

  • [x] app/jobs/edge_learner_job.rb -- verified: does not exist yet (new file). Reference pattern exists at app/jobs/pregame_threshold_job.rb (23 lines, delegates to bot service in perform method)
  • [x] app/jobs/price_update_job.rb -- verified: exists (21 lines), is a stub with empty perform body and comment "Stub -- bot consumers implement in Sprint 4". Header confirms consumers: "Late-Game Lock, Edge Learner"
  • [x] app/jobs/time_update_job.rb -- verified: exists (19 lines), is a stub with empty perform body and comment "Stub -- bot consumers implement in Sprint 4". Header confirms consumers: "Late-Game Lock, Edge Learner"
  • [x] app/jobs/sweep_eligible_job.rb -- verified: exists (27 lines), fans out to BulkSweepJob only, has comment "Edge Learner will be added in Sprint 4 (#29)". Fan-out pattern is established: BulkSweepJob.perform_later(...)
  • [x] test/jobs/edge_learner_job_test.rb -- verified: does not exist yet (new file)

Repo Placement

OK. Issue filed on ldraney/prediction-assistant, all file targets are in this repo. Single repo scope.

Dependencies

  • GroupBuilder service (#69) -- board item #1750, backlog column. app/services/group_builder.rb does not exist. Hard dependency for AC4. Issue documents this: "Depends on GroupBuilder service (sub-ticket)".
  • PricePattern model (#67) -- board item #1748, backlog column. app/models/price_pattern.rb does not exist. Hard dependency for Constraints ("Closed trade results must be fed back into PricePattern"). Issue documents this.
  • PositionTracker (#24) -- board item #1711, validation column. app/services/position_tracker.rb EXISTS. Required for AC3 exit logic.
  • OrderService (#23) -- board item #1710, validation column. app/services/order_service.rb EXISTS. Required for AC5.
  • EdgeLearnerConfig model -- EXISTS at app/models/edge_learner_config.rb. All referenced config keys verified: risk_level, bet_size_per_group, max_portfolio_exposure, stop_loss, take_profit, learning_sensitivity.
  • ActivityLog model -- EXISTS at app/models/activity_log.rb. Current ACTIONS: place_order, cancel_order, fill, partial_fill, error, dry_run, bulk_sweep. Agent will need to add Edge Learner-specific actions (group_generation, pattern_detection) during implementation.
  • Late-Game Lock -- board items #1747 and #1689, both in todo column. Also targets PriceUpdateJob and TimeUpdateJob. Issue correctly states: "Must merge AFTER Late-Game Lock PR to avoid Solid Queue job conflicts."
Prior review concern resolved: Previous review recommended adding explicit ordering and merge coordination notes. These sections already exist in the issue body: Dependencies section states "Must merge AFTER Late-Game Lock" and Merge Coordination section gives explicit merge order "PricePattern -> GroupBuilder -> EdgeLearnerJob".

Acceptance Criteria

5 AC items, all testable with caveats:
  • AC1: Testable -- consumer pattern well-established from PregameThresholdJob
  • AC2: Testable -- mechanical fan-out additions (add EdgeLearnerJob.perform_later(...) to 3 jobs)
  • AC3: Testable -- complex but well-specified. 3 risk-level exit strategies (low=immediate, medium=trailing stop, high=take-profit). Requires PositionTracker (EXISTS) and OrderService (EXISTS)
  • AC4: Testable only after GroupBuilder (#69) merges -- delegates to GroupBuilder for group generation. Hard dependency documented in issue.
  • AC5: Testable -- standard ActivityLog pattern
Minor gap: No AC covers specific time_update event handling behavior beyond AC1's "processes time_update payloads". Constraints mention "Learning model reversal signal overrides risk-level hold rules" but no AC explicitly tests this override.

Blast Radius

Low risk. All fan-out changes are additive:
  • PriceUpdateJob and TimeUpdateJob are stubs -- adding fan-out is their first real consumer wiring
  • SweepEligibleJob already fans out to BulkSweepJob -- adding EdgeLearnerJob follows the identical pattern
  • MarketScannerJob (the event source) is not modified -- it already emits all 3 event types
  • Late-Game Lock also targets PriceUpdateJob/TimeUpdateJob but merge order is documented

Decomposition Assessment

5 file targets in 1 repo, 5 acceptance criteria, follows established PregameThresholdJob consumer pattern. Fan-out changes are mechanical (3 one-line additions). The new job is the most complex piece but has clear AC and a reference implementation. No decomposition needed.

Recommendation

All checks pass. The sole prior blocker ([SCOPE] arch-rails note missing) was a false positive -- the note exists at slug arch-rails in pal-e-docs, confirmed by multiple agents. Prior [BODY] recommendations from first review were resolved in the current issue body. Ticket is ready for development.