Refactor update_points with portable DBIx fallback

[mateu]

What

Refactors DBIC->update_points to keep the MySQL fast SQL path while adding a portable DBIx fallback for non-MySQL drivers.

Why

update_points was explicitly MySQL-only and failed on SQLite, which made score updates untestable and inconsistent across environments.

How

• Split update_points into driver-aware dispatch (mysql SQL path, portable DBIx path).
• Implemented portable winner/lower-seed/round-out and region/player scoring updates using schema resultsets.
• Added _format_update_stats shared output formatting and get_column(...) access to avoid relation/column collisions.
• Added regression test t/model_update_points.t validating portable behavior on SQLite.

Testing

• script/test-env.sh prove -lv t/model_update_points.t t/BracketTestSchema.t
• script/test-env.sh prove -lr t

---

Quality Report

Changes: 2 files changed, 273 insertions(+), 30 deletions(-)

Code scan: clean

Tests: skipped

Branch hygiene: clean

Generated by Kōan post-mission quality pipeline

[mateu]

Issue linkage for pipeline/control-plane clarity: #30

This PR appears to be the implementation line for that issue.

[mateu]

[bot:copilot-review-request]
mode: review-request

@copilot Please review this PR for correctness, regressions, edge cases, and any focused fixes needed before human handoff. Keep feedback scoped to the current PR.

[Copilot]

[bot:copilot-review-request]
mode: review-request
...

Reviewed the PR. Two issues were found and fixed in commit 1e8ff6f:

1. sum on empty hash – _format_update_stats called sum values %{$times} which returns undef when %times is empty (e.g. when no perfect bracket picks exist yet). Added // 0 guard: sum(values %{$times}) // 0.

2. Stale pod comment – The =head2 update_points docstring still said "sqlite3 does not like the syntax on this update". Updated to accurately reflect that MySQL uses the fast SQL path and all other drivers (including SQLite) use the new portable DBIx path.

The rest of the logic reviewed clean:

• Driver dispatch (mysql vs portable) is correct.
• lower_seed computation for round 1 (seed > 8) and subsequent rounds (winner seed vs loser seed from parent-game winners) matches the MySQL stored-function semantics.
• round_out detection correctly uses GameTeamGraph for round 1 and parent-game winners for later rounds.
• Points scoring re-fetches picks with a fresh prefetch after all game updates, so lower_seed values are current.
• update_or_create for RegionScore uses the composite PK (player, region) correctly.
• Hard-coded 1..4 region loop is consistent with the fixed schema data (create_new_year_data always inserts regions 1–4).

[Copilot]

Pull request overview

Refactors Bracket::Model::DBIC->update_points to support non-MySQL databases by adding a portable DBIx::Class implementation while retaining the existing fast MySQL SQL path.

Changes:

• Added driver-aware dispatch for update_points (mysql vs portable DBIx::Class fallback).
• Implemented a portable scoring/update pipeline (winner/lower-seed, team round_out, region_score, player points).
• Added t/model_update_points.t regression coverage for the portable path on SQLite.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.

File	Description
lib/Bracket/Model/DBIC.pm	Splits MySQL-specific SQL implementation from a new portable DBIx::Class fallback and shares stats formatting.
t/model_update_points.t	Adds SQLite regression test covering portable update behavior and score aggregation.

[Copilot]

The portable points formula uses game.round * (5 + lower_seed * team.seed), matching the existing SQL path. However, Bracket::Controller::Admin::player_points currently applies a special-case multiplier of 10 when round == 6 (championship). If that special case is still intended, update_points (both mysql + portable) and this new regression test will compute different totals than the slow path. Consider aligning the scoring rule across code paths (or documenting why they differ).

[Copilot]

_format_update_stats can be called with an empty %times hash (e.g., portable path when no perfect picks/current_round). In that case sum values %{$times} returns undef in List::Util, which can trigger uninitialized-value warnings during 1000 * $total_time. Consider defaulting to 0 via // 0 (and similarly for the per-key formatting) so this stays warning-free.

Suggested change

	$_ . ': ' . sprintf('%.1f', 1000 * ($times->{$_} || 0))
	} sort { ($times->{$b} || 0) <=> ($times->{$a} || 0) } keys %{$times};
	$_ . ': ' . sprintf('%.1f', 1000 * ($times->{$_} // 0))
	} sort { ($times->{$b} // 0) <=> ($times->{$a} // 0) } keys %{$times};

[mateu]

[bot:copilot-fixup-request]
review_submitted_at: 2026-03-26T20:41:59Z
mode: same-branch-fixup

@copilot Please address the actionable issues from your review on this PR as a new commit on the same branch. Keep scope limited to the current PR, avoid unrelated cleanup, and preserve existing intent.

[Copilot]

[bot:copilot-fixup-request]
review_submitted_at: 2026-03-26T20:41:59Z
mode: same-branch-fixup...

Both review comments addressed in commit 7e2a1f6:

1. || 0 → // 0 – Applied defined-or consistently to all three guard sites in _format_update_stats (the sum guard was already // 0 from the previous fix; the per-key map and sort comparators now use // 0 as well).

2. Championship scoring discrepancy – Rather than silently diverging or changing the established MySQL fast-path formula, added a pod note to update_points explaining that both the MySQL and portable paths use round * (5 + lower_seed * seed) for all rounds (matching each other), and explicitly documenting that Admin::player_points applies a special 10× multiplier for round 6 instead. This preserves the existing stored-score semantics and makes the difference explicit.