Review: EdgeLearnerJob consumer + fan-out wiring
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-railsnote 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 atapp/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.rbdoes 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.rbdoes 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.rbEXISTS. Required for AC3 exit logic. - OrderService (#23) -- board item #1710, validation column.
app/services/order_service.rbEXISTS. 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.