pal-e-docs

Review: GroupBuilder service -- combinatorial market group generation

review-1750-2026-07-04 Review

review ready
Project

Verdict: READY

Template Completeness

  • [x] Type -- Feature
  • [x] Lineage -- Sprint 6, sub-ticket of #29
  • [x] Repo -- ldraney/prediction-assistant
  • [x] User Story -- AI Portfolio Builder, Edge Learner group evaluation engine
  • [x] Context -- detailed background on Group Builder sub-system
  • [x] File Targets -- 2 new files listed
  • [x] Feature Flag -- None (internal service, appropriate)
  • [x] Acceptance Criteria -- 5 criteria
  • [x] Test Expectations -- 4 test areas
  • [x] Constraints -- 3 constraints listed
  • [x] Checklist -- standard 3-item checklist
  • [x] Related -- parent, consumer, config references

Traceability

  • [x] story:portfolio-builder label -- AI Portfolio Builder
  • [x] story note verified -- found in project-prediction-assistant user-stories section, links to story-prediction-assistant-portfolio-builder
  • [x] arch:rails label -- Rails component
  • [x] arch note verified -- arch-rails note exists in pal-e-docs (false positive in prior review; confirmed by multiple agents)
  • [x] Forgejo issue -- #69, open

File Targets

  • [x] app/services/group_builder.rb -- verified: new file, parent directory app/services/ exists with 5 existing services (kalshi_client.rb, order_service.rb, position_tracker.rb, pregame_stacker_bot.rb, sizing_engine.rb)
  • [x] test/services/group_builder_test.rb -- verified: new file, parent directory test/services/ exists with 5 existing test files

Repo Placement

OK -- issue filed on ldraney/prediction-assistant, all work scoped to the same repo. Single-repo change, no cross-repo concerns.

Dependencies

  • Parent: #29 (Edge Learner bot) -- board item 1716, backlog, sprint:6
  • Consumed by: EdgeLearnerJob (separate sub-ticket of #29, not yet created or in backlog)
  • Config: EdgeLearnerConfig max_group_size field exists (line 7 of edge_learner_config.rb) -- but currently validated as positive integer. Issue spec says it should be enum ("pairs"/"triples"/"uncapped"). The issue acknowledges this in Related section: "fix tracked in separate bug sub-ticket". This is a known dependency, not a scoping failure.
  • No blocking items in in_progress column.
  • Pure logic service with zero database access -- no migration or schema dependency.

Acceptance Criteria

5 criteria, all verifiable by an agent:
  • AC1: Interface contract (GroupBuilder.new(markets, max_group_size:).call) -- testable with unit test
  • AC2: Exact group counts (3=7, 4=15, 5=31) -- testable with assertions
  • AC3: max_group_size filtering -- testable with enum boundary tests
  • AC4: Combined probability calculation -- testable with known inputs
  • AC5: Efficiency for 10 markets (1023 groups) -- testable with timing benchmark
No missing criteria. Test expectations align with ACs. No test commands specified but the test file path is clear and follows existing conventions.

Blast Radius

Minimal. GroupBuilder is a new standalone service consumed only by EdgeLearnerJob (not yet built). No existing services reference GroupBuilder or use combinatorial logic. The only related pattern is sizing_engine.rb which uses "combination" in a different context (position sizing). No downstream consumers affected.

Decomposition Assessment

2 file targets, 1 repo, 5 acceptance criteria. Pure logic service with no external dependencies. Estimated agent work: under 5 minutes. No decomposition needed.

Recommendation

All checks pass. The prior NEEDS_REFINEMENT verdict was based on a false positive: the arch-rails architecture note does exist at slug arch-rails in pal-e-docs (confirmed by multiple agents). No remaining scope issues.