Refactor standard count chart helpers

[Mattsface]

Summary

Implements PR B for #44 by consolidating repeated Plotly mechanics across the standard per-game count charts while keeping baseball-specific semantics explicit.

• shares raw observation trace construction
• shares rolling-average trace construction
• shares team/MLB reference-line construction and annotation behavior
• shares the common zero-anchored count-chart layout
• keeps Hits, Batting Strikeouts, Runs, Baserunners, and Hits Allowed as explicit public builders
• keeps Pitching, Run Differential, and Normalized Comparison structurally unchanged
• strengthens Hits Allowed chart regression coverage
• updates architecture documentation to reflect the new chart abstraction boundary

Scope intentionally excluded

• route changes
• analytics/formula changes
• Pydantic validator cleanup
• persistence or ingestion changes
• template/static changes
• Player UI
• generic MetricConfig/ChartConfig frameworks

Validation

Branch review found no code blocker. PR CI should be treated as the authoritative full pytest/Ruff validation before merge.

Refs #44

[Mattsface]

@claude review please

[claude]

Claude finished @Mattsface's task in 1m 20s —— View job

---

Review: Refactor standard count chart helpers

• Gather context (diff, changed files, AGENTS.md)
• Review app/web/charts.py refactor
• Review tests/test_hits_allowed.py coverage
• Review docs/architecture-deep-dive.md updates
• Post review feedback

Summary: This is a clean, well-scoped refactor. I diffed app/web/charts.py against origin/main function-by-function and confirmed the extraction is behavior-preserving — every trace construction, hover template, reference-line labelling rule, and layout option in build_team_hits_figure, build_team_strikeouts_figure, build_team_runs_figure, build_team_baserunners_figure, and build_team_hits_allowed_figure now routes through the four new private helpers (_add_standard_raw_trace, _add_standard_rolling_trace, _add_standard_reference_lines, _apply_standard_count_chart_layout) with no logic changes, only mechanical extraction.

Specific things I checked:

• Reference-line labelling (app/web/charts.py:263-275): the shared helper correctly reproduces the old per-builder "label the MLB line if present, otherwise label the team line" rule (only one label to avoid overlap when both lines are close).
• build_team_hits_allowed_figure (app/web/charts.py:625-699): identical mechanics to build_team_hits_figure, matching the module docstring's "mirror of the hits chart" explanation.
• Pitching, run differential, and normalized comparison builders are untouched — confirmed via git diff hunks, matching the PR's stated exclusion of those charts (they have genuinely distinct axis/series semantics, e.g. run differential's non-zero-anchored signed axis).
• tests/test_hits_allowed.py: the added assertions (app/web/charts.py-facing trace-name ordering, annotation text, reference-line values/dash style) meaningfully strengthen regression coverage for the shared helper path rather than just padding test count — good.
• Docs: docs/architecture-deep-dive.md update accurately describes the new shared-helper boundary and correctly preserves the historical rationale for keeping public builders explicit rather than introducing a generic MetricConfig-style framework, consistent with AGENTS.md's abstraction guidance.

This matches the repo's architectural principles well: boring, explicit public builders retained, shared mechanics factored into small named helpers rather than a generic engine, no dependency-direction changes, no scope creep (route/analytics/persistence untouched, as stated in the PR description).

Nitpicks (non-blocking):

• hover_data: list[tuple[object, ...]] in the new helper signatures (app/web/charts.py:184, 213) is a reasonable typing choice for the genuinely heterogeneous hover tuples (mixed str/int/float per metric), but it's worth a second look if a future metric's hover tuple needs stronger typing than object.

Validation note: I wasn't able to run ruff/pytest in this sandboxed environment (poetry/ruff not on PATH here), so I relied on direct diff/behavior comparison against origin/main instead. The PR description already flags this and defers to CI as the authoritative check — I'd treat CI green as required before merge, per AGENTS.md's "Definition of done."

No blocking issues found.