Review: Bot detail trade history table + pagination

review-1763-2026-07-04 Review

review approved

Verdict: APPROVED

Template Completeness

  • [x] Type -- Feature
  • [x] Lineage -- Sub-issue of #84, decomposition documented
  • [x] Repo -- ldraney/prediction-assistant
  • [x] User Story -- As a/I want/So that format, clear
  • [x] Context -- Sufficient background, references existing show page layout and dependency
  • [x] File Targets -- 3 files (1 create, 2 modify), plus "NOT to touch" list
  • [x] Feature Flag -- "None" (appropriate for internal display feature)
  • [x] Acceptance Criteria -- 5 items, all testable
  • [x] Test Expectations -- 4 test cases + run command
  • [x] Constraints -- 4 constraints including Turbo Frame requirement and responsive design
  • [x] Checklist -- Present
  • [x] Related -- 4 related issues documented

Traceability

  • [x] story:portfolio-builder label -- AI Portfolio Builder
  • [x] story note verified -- found in project-prediction-assistant user-stories section (key: portfolio-builder, role: Trader (Lucas))
  • [x] arch:rails label -- present on board item
  • [ ] arch note MISSING -- [SCOPE] No arch-rails note found in pal-e-docs. This is systemic across many board items sharing this label. Not a blocker for this ticket.
  • [x] Forgejo issue -- https://forgejo.tail5b443a.ts.net/ldraney/prediction-assistant/issues/87, open

File Targets

  • [x] app/views/bots/_trade_history.html.erb -- new partial to create. Does not exist yet (correct).
  • [x] app/controllers/bots_controller.rb -- verified: exists, show action at line 26, already loads strategy and activity logs. Extension point is clear.
  • [x] test/controllers/bots_controller_test.rb -- verified: exists with 589 lines of existing tests covering index, show, update, toggle, activity. New trade history tests fit naturally.

Repo Placement

OK. Issue filed on ldraney/prediction-assistant, work targets the same repo. Single-repo scope.

Dependencies

  • #82 (SimulationResult model + P&L aggregation service) -- Hard dependency, correctly documented in Context section. Board item #1758 is currently in backlog. SimulationResult model does not exist in the codebase yet. This ticket must not start until #82 is merged.
  • #84 (parent issue) -- Decomposed, this ticket is one of two sub-issues.
  • #86 (P&L chart + summary stats) -- Sibling sub-issue, independent scope. Correctly listed in "Files NOT to touch."
  • #83 (Bot card performance indicators) -- Related but independent.

Acceptance Criteria

5 acceptance criteria, all verifiable by automated tests:
  • Table rendering -- assert_select for partial presence
  • Column list -- assert_select for column headers
  • Pagination at 10 per page -- controller test with count assertion
  • Empty state message -- controller test with message text assertion
  • Turbo Frame pagination -- test response includes turbo-frame element
Test expectations (4) align well with AC. Run command provided: bin/rails test test/controllers/bots_controller_test.rb.

Blast Radius

Low. The existing codebase already has a Turbo Frame pagination pattern in _activity_feed.html.erb (cursor-based with "Load older entries"). The new trade history adds a second pagination mechanism (offset/limit) on the same page. The ticket explicitly calls out using offset/limit rather than cursor, which is appropriate for a trade history table where users navigate forward and backward. No downstream consumers are affected. The change is additive -- new partial, minor controller extension.

Decomposition Assessment

3 file targets in 1 repo, 5 acceptance criteria, 4 test expectations. Estimated agent work well under 5 minutes. No decomposition needed.

Recommendation

  • [SCOPE] Create architecture note arch-rails for the Rails application component. This is systemic across many board items and not specific to this ticket.
No other action needed. Scope is solid, file targets verified, dependencies documented, acceptance criteria testable.