pal-e-docs

Review: Fix EdgeLearnerConfig max_group_size validation

review-1753-2026-07-04-v2 Review

review needs-refinement
Project

Verdict: NEEDS_REFINEMENT

Template Completeness

  • [x] Type -- Bug
  • [x] Lineage -- Sprint 6, sub-ticket of #29, pre-existing from #3
  • [x] Repo -- ldraney/prediction-assistant
  • [x] What Broke -- max_group_size validated as positive integer, should be enum
  • [x] Repro Steps -- 4-step reproduction path
  • [x] Expected Behavior -- string enum values should pass validation
  • [x] Environment -- file, line, commit, spec reference all present
  • [x] Acceptance Criteria -- 5 criteria listed
  • [x] Related -- parent, introduced-by, and downstream consumer identified

Traceability

  • [x] story:portfolio-builder label -- AI Portfolio Builder
  • [x] story note verified -- found in project-prediction-assistant user-stories section
  • [x] arch:rails label -- Rails component
  • [ ] arch note MISSING -- [SCOPE] search for "arch-rails" returned no results. Rails is the project's foundational framework; acceptable as foundational infrastructure if arch notes are only created for custom components.
  • [x] Forgejo issue -- ldraney/prediction-assistant#72, open

File Targets

  • [x] app/models/edge_learner_config.rb, line 24 -- verified: validate_positive_integer("max_group_size") is exactly as described
  • [x] docs/bots/edge-learner.md, line 72 -- verified: max_group_size | enum | "triples" with values "pairs", "triples", "uncapped"
  • [x] app/models/strategy.rb, line 73 -- verified: validate_inclusion(key, allowed) method exists in base class, ready to use
  • [x] app/models/bot_catalog.rb, line 248 -- verified: max_group_size defined as type: :number, default: 3 in CONFIG_SCHEMAS (needs change to type: :select)

Repo Placement

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

Dependencies

  • Parent ticket #29 (Edge Learner bot, board item #1716) is in backlog. This bug fix is independent and can proceed without the parent.
  • GroupBuilder service referenced in "Related" does not exist yet -- it is part of the parent feature. No downstream consumer to break.
  • No blocking dependencies. No items in in_progress that block this.

Acceptance Criteria

  • [x] AC 1 -- Testable: swap validate_positive_integer for validate_inclusion with %w[pairs triples uncapped]
  • [x] AC 2 -- Testable: unit tests for valid/invalid values
  • [ ] AC 3 -- INACCURATE: says "BotCatalog::ENTRIES config_fields" but the config field definitions live in BotCatalog::CONFIG_SCHEMAS, not ENTRIES. The ENTRIES array holds display metadata (Entry structs). The actual change needed is at line 248 in CONFIG_SCHEMAS: change type: :number, default: 3 to type: :select, default: "triples" with options for pairs/triples/uncapped.
  • [ ] AC 4 -- INACCURATE: says "Test fixtures in test/fixtures/" but there are no EdgeLearner YAML fixture files. The test data with integer max_group_size values lives in test helper methods: test/models/strategy_test.rb:47 ("max_group_size" => 4), test/controllers/bots_controller_test.rb:53 ("max_group_size" => 3), and test/services/position_tracker_test.rb:43 ("max_group_size" => 4). These test files WILL need modification (changing integer values to enum strings), which contradicts "without modification".
  • [x] AC 5 -- Likely not needed: config is stored as JSON/JSONB, so string values work without schema migration. Correct to include as conditional.

Blast Radius

  • validate_positive_integer is used in 5 other places across LateGameLockConfig, StackerConfig, and BulkSweepConfig -- all correct for their integer fields, no similar enum mismatch found.
  • max_group_size with integer values appears in 3 test files (strategy_test, bots_controller_test, position_tracker_test) -- all need updating to enum strings.
  • GroupBuilder service does not exist yet, so no runtime downstream breakage.
  • BotCatalog CONFIG_SCHEMAS renders the form field -- changing type from :number to :select will affect the bot detail page form rendering. The form rendering code should already handle :select type (used by risk_level, time_remaining_threshold, etc.).

Decomposition Assessment

3 source files + 3 test files in 1 repo. 5 AC (at threshold). Estimated agent time: under 5 minutes -- this is a mechanical validation swap, catalog field type change, and test data update. No decomposition needed.

Recommendations

  • [BODY] AC 3: Change "BotCatalog::ENTRIES config_fields for the affected bot(s) are updated" to "BotCatalog::CONFIG_SCHEMAS entry for EdgeLearnerConfig is updated -- max_group_size field changes from type: :number, default: 3 to type: :select, default: 'triples' with options pairs/triples/uncapped"
  • [BODY] AC 4: Change "Test fixtures in test/fixtures/ are updated to reflect the config model changes -- all existing tests pass without modification" to "Test helper methods in strategy_test.rb, bots_controller_test.rb, and position_tracker_test.rb are updated to use enum string values ('triples' or 'pairs') instead of integers for max_group_size -- all tests pass"
  • [SCOPE] arch:rails note missing -- create architecture note arch-rails if the convention requires arch notes for framework-level technology choices, or document that foundational framework labels do not require backing notes.