Skip to content

docs(sports): record where B6 stands, and why it is waiting - #435

Open
ChuckBuilds wants to merge 3 commits into
mainfrom
docs/b6-decision-of-record
Open

docs(sports): record where B6 stands, and why it is waiting#435
ChuckBuilds wants to merge 3 commits into
mainfrom
docs/b6-decision-of-record

Conversation

@ChuckBuilds

@ChuckBuilds ChuckBuilds commented Aug 5, 2026

Copy link
Copy Markdown
Owner

The phase table had B4 as "next" and B5 as "after B4" while both had shipped, and described B6 as blocked on B4's gate — which is now merged and released. A plan that misreports which phase it is in is worse than no plan: the next person reads it and repeats finished work.

Corrected, and three things that were only ever decided in conversation are now written down.

B6 is deliberately held

3.2.0 was published 2026-08-03; 3.1.0 ran nine months before it. B6's premise is that cores without src.common.sports_scroll are gone, and there is no release-asset count or install telemetry to demonstrate that. Running it now strands users on their current plugin versions.

The gate that makes B6 safe is already built and tested — it refuses installs whose floor exceeds the core version, and refuses floors above 2.0.0 on a core reporting an untrustworthy version. It is the calendar that is missing, and no amount of further code changes that. Also records the traps for whoever runs it: four plugins declare their floor top-level where editing versions[0] is a silent no-op, and the floor has three live spellings.

Stop adopting further shared modules

data_sources.py (9 copies), game_renderer.py (8), base_odds_manager.py. Each adoption adds a second copy to keep in step against a payoff contingent on B6.

A B5 retrospective

Because "the adoption went fine" is not what happened — four of eight plugins shipped with scroll mode broken on a 3.2.0 core (repaired in plugins #251, tidied in #252). Three points worth carrying forward:

  • The bundled fallback did not protect anyone from this. The break was on the modern path, which the fallback never touches. That is an argument for the sunset, not against it.
  • Every gate was green. The harness renders scoreboard screens, not scroll mode; the AST check looked for unresolved globals and self.NHL_SEPARATOR_ICON is an attribute read. The fix was to stop reasoning about source and build the object.
  • The ledger: 5,685 lines before adoption → 10,610 after → ~8,620 once the dead duplication was removed → ~3,300 after B6. Until B6 runs the adoption is net negative on disk, with one delivered user-visible gain (native target_fps).

Also replaces the "what's next" list, whose first five items were all done, with what actually remains: the stale plugin-test tranche, hardware soaks for the six unsoaked adoptions, and a CLAUDE.md panel-size fix.

Documentation only — no code changes.

🤖 Generated with Claude Code

https://claude.ai/code/session_01Udr6MfaFLUPhX5Fgo67Jf5

Summary by CodeRabbit

  • Documentation
    • Updated rollout tracking to show B4 and B5 as complete.
    • Documented the rationale for keeping B6 blocked pending adoption evidence.
    • Added details on known adoption issues, corrective testing, storage impact, and remaining validation work.

The phase table had B4 as "next" and B5 as "after B4" while both had shipped,
and described B6 as blocked on B4's gate — which is now merged and released. A
plan that misreports which phase it is in is worse than no plan: the next
person reads it and repeats finished work.

Corrected, and three things that were only ever decided in conversation are now
written down:

  * **B6 is deliberately held.** 3.2.0 published 2026-08-03; 3.1.0 ran nine
    months before it. B6's premise is that cores without the module are gone,
    and there is no release-asset count or install telemetry to show that.
    Running it now strands users on their current plugin versions. The gate
    that makes it safe is already built and tested — it is the calendar that is
    missing, and no amount of further code changes that.
  * **Stop adopting further shared modules** (data_sources, game_renderer,
    base_odds_manager) until B6 closes. Each adoption adds a copy to keep in
    step against a payoff contingent on B6.
  * **A B5 retrospective**, because "the adoption went fine" is not what
    happened: four of eight shipped with scroll mode broken on a 3.2.0 core.
    The bundled fallback did not protect against it — the break was on the
    modern path — which is an argument for the sunset, not against it. Records
    the ledger too: net negative on disk until B6 runs.

Also replaces the "what's next" list, whose first five items were all done,
with what actually remains.

Co-Authored-By: Claude Opus 5 (1M context) <[email protected]>
Claude-Session: https://claude.ai/code/session_01Udr6MfaFLUPhX5Fgo67Jf5
@coderabbitai

coderabbitai Bot commented Aug 5, 2026

Copy link
Copy Markdown

Review Change Stack

Warning

Review limit reached

@ChuckBuilds, you've reached your PR review limit, so we couldn't start this review.

Next review available in: 56 minutes

You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository.

How can I continue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews.

How do review limits work?

CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability.

For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window.

Please refer docs for additional details.

Review details
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: ca0741a5-5209-4dae-a5bd-4d3d47b816f8

📥 Commits

Reviewing files that changed from the base of the PR and between df8f69a and ed2b81a.

📒 Files selected for processing (1)
  • docs/SPORTS_UNIFICATION.md
📝 Walkthrough

Walkthrough

The rollout documentation now records B4 and B5 completion, keeps B6 blocked pending 3.2.0 adoption evidence, documents B5 defects and corrective validation, and lists the remaining work.

Changes

Sports unification rollout

Layer / File(s) Summary
Rollout status and B6 decision
docs/SPORTS_UNIFICATION.md
B4 and B5 are marked complete. B6 remains blocked until adoption evidence is available. The document records compatibility protections, unblock criteria, and manifest requirements.
Adoption retrospective and remaining work
docs/SPORTS_UNIFICATION.md
The B5 retrospective records scroll-mode defects, instance-state dependencies, object-level validation, line-count updates, and target_fps behavior. Further module adoption is paused. Remaining work now includes B6 monitoring, stale tests, hardware soak testing, documentation correction, and compatibility regression coverage.

Estimated code review effort: 1 (Trivial) | ~5 minutes

Possibly related PRs

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly identifies the documentation change and the reason B6 remains pending.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch docs/b6-decision-of-record

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@codacy-production

Copy link
Copy Markdown

Up to standards ✅

🟢 Issues 0 issues

Results:
0 new issues

View in Codacy

NEW Get contextual insights on your PRs based on Codacy's metrics, along with PR and Jira context, without leaving GitHub. Enable AI reviewer
TIP This summary will be updated as you push new changes.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 2

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@docs/SPORTS_UNIFICATION.md`:
- Around line 361-367: Update the earlier B5 safety claim in
docs/SPORTS_UNIFICATION.md to promise only fallback compatibility on cores
without the shared module, not immunity from runtime regressions during
adoption. Retain the documented requirement for object-level and scroll-mode
validation to verify full behavior.
- Line 409: Fix the ATX heading syntax in the documentation around the manifest
entry text by adding the required space after the heading marker, preserving the
existing heading content.
🪄 Autofix

✅ Autofix completed


ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: a54a3e92-1a8b-4421-bb8f-67ad616495e4

📥 Commits

Reviewing files that changed from the base of the PR and between d305be6 and df8f69a.

📒 Files selected for processing (1)
  • docs/SPORTS_UNIFICATION.md

Comment thread docs/SPORTS_UNIFICATION.md
Comment thread docs/SPORTS_UNIFICATION.md Outdated
Steps 1–5 of the original plan are **done**: 3.2.0 is tagged and published with
a version number CI now asserts (#428), the compatibility gate is in
`install_plugin` and reads `compatible_versions` as well as the floor (#431,
#433), the newest manifest entry is required to use `ledmatrix_min_version`

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail
sed -n '404,412p' docs/SPORTS_UNIFICATION.md
markdownlint-cli2 docs/SPORTS_UNIFICATION.md

Repository: ChuckBuilds/LEDMatrix

Length of output: 9028


Add the missing space to the ATX heading on line 409.

markdownlint-cli2 reports MD018 for line 409 (#433), the newest manifest ent...). Change it to #433) so it is valid Markdown heading syntax.

🧰 Tools
🪛 markdownlint-cli2 (0.23.2)

[warning] 409-409: No space after hash on atx style heading

(MD018, no-missing-space-atx)

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@docs/SPORTS_UNIFICATION.md` at line 409, Fix the ATX heading syntax in the
documentation around the manifest entry text by adding the required space after
the heading marker, preserving the existing heading content.

Source: Linters/SAST tools

@coderabbitai

coderabbitai Bot commented Aug 5, 2026

Copy link
Copy Markdown

Fixes Applied Successfully

Fixed 1 file(s) based on 2 unresolved review comments.

Files modified:

  • docs/SPORTS_UNIFICATION.md

Commit: ac44b5a6f558c8dea27070bf5015c1eb3861960b

The changes have been pushed to the docs/b6-decision-of-record branch.

Time taken: 3m 10s

coderabbitai Bot and others added 2 commits August 5, 2026 20:59
Fixed 1 file(s) based on 2 unresolved review comments.

Co-authored-by: CodeRabbit <[email protected]>
A line wrapped onto "#433), the newest manifest entry ...", which
markdownlint reads as a malformed ATX heading (MD018). Reflowed so the
line starts with "(#431, #433)" instead.

Not the suggested fix: adding a space after the hash would have turned
the PR reference into "# 433". The B5 safety claim raised alongside this
was already corrected in ac44b5a.

Co-Authored-By: Claude Opus 5 (1M context) <[email protected]>
Claude-Session: https://claude.ai/code/session_01Udr6MfaFLUPhX5Fgo67Jf5
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant