Skip to content

Five things the stack merged with nobody having read it - #37

Merged
HarryCordewener merged 1 commit into
mainfrom
fix/post-merge-cleanup
Aug 15, 2026
Merged

Five things the stack merged with nobody having read it#37
HarryCordewener merged 1 commit into
mainfrom
fix/post-merge-cleanup

Conversation

@HarryCordewener

Copy link
Copy Markdown
Member

Eleven PRs landed on main this session and ten of them were never reviewed: CodeRabbit skips a base branch that is not the default, and every manual @coderabbitai review retrigger came back rate-limited. These are the defects reading the diffs afterwards turned up. All five were already on main before this branch.

Fix Where Was
Listing glyph follows provenance Games.razor hard-coded state-present ● beside a ◇ declared chip — one count described two ways at once
badge.json keeps its 5 minutes BadgeEndpoints.cs, ApiResponse.cs Cache-Control assigned after WriteJsonAsync started the response, so it was dropped and the route shipped the API default max-age=60
RevokeAsync gets its doc back ClaimService.cs ResignAsync was inserted between the comment and the method, attaching "never called because a beacon went missing" to the wrong one
MSSP counts read invariantly, non-negative MsspLint.cs current-culture int.TryParse1,024 passed or failed depending on the host, -3 read as well-formed
Opt-out table keeps its third route design spec §11 the Request row sat below the paragraph that ended the table, so the rendered spec dropped one of the three routes it documents

Two of these are behavioural and are covered by new tests; the badge one is a good illustration of why it survived review-by-CI — ABadgeIsCacheableAndRevalidates asserted max-age=300 on the SVG route only, and the JSON route beside it was never looked at. Both are asserted now.

The SVG route also stopped hand-rolling Prepare/NotModified/write and goes through ApiResponse.WriteAsync like everything else, which drops a triplicated ETag.Of(body).

Verification

dotnet build MUIndex.slnx -c Release      → 0 warnings, 0 errors
MUI.Web.Tests        348 passed
MUI.Catalog.Tests    300 passed  (real Postgres, 0 skipped)
MUI.Crawl.Tests      142 passed
MUI.Crawler.Tests    156 passed
MUI.Discovery.Tests  264 passed
                    ---- 1010 total, 0 failed, 0 skipped

Not in this PR

Found in the same audit, left alone because each is its own change:

  • The presence rollup Roll an hour up without letting it lose one of its three states #28 built is write-only. NpgsqlPresenceRollupStore's only production consumer is PresenceMaintenance, which writes it; the heatmap, activity band and rankings all still SELECT … FROM presence_sample. Until a reader lands, PresenceRetentionOptions.AsDesigned buys nothing on any surface.
  • §8.5's owner-dashboard opt-out toggle, and a public opt-out request form — the "recorded request" channel is CLI-only today.
  • AboutIdentity.ContactConfigured is false on the built-in InfoUrl, so the about page tells operators to write to somebody and names nobody.
  • §8.5's WHO-format override.

🤖 Generated with Claude Code

Eleven PRs landed on main and ten of them were never reviewed — CodeRabbit
skips a non-default base branch and every manual retrigger came back rate
limited. These are what reading the diffs afterwards turned up.

- The listing row printed a hard-coded measured dot beside the provenance
  chip that says declared, so one count was described two ways in the same
  breath. Game.razor was fixed for exactly this and the listing pointing at
  it was not.

- badge.json set its five-minute Cache-Control after WriteJsonAsync had
  already started the response, so the header was dropped and the route
  shipped the API's default minute. The lifetime is now passed into the
  write rather than assigned after it, and the SVG route goes through the
  same helper instead of hand-rolling Prepare/NotModified. Only the SVG
  route was asserted on, which is why nothing caught it; both are tested now.

- ResignAsync was inserted between RevokeAsync's doc comment and RevokeAsync,
  leaving revocation undocumented and attaching "never called because a
  beacon went missing" to the wrong method — the one piece of doc that stops
  someone wiring revocation to beacon loss.

- MsspLint parsed counts in the current culture and accepted negatives, so
  "1,024" was a pass or a fault depending on the host and "-3" read as a
  well-formed answer. A scorecard that disagrees with itself across machines
  is worse than one that is strict.

- The opt-out table in the spec had its Request row stranded below the
  paragraph that ended the table, so the rendered spec silently dropped one
  of the three routes it documents.

1010 tests across five suites, Postgres exercised.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@coderabbitai

coderabbitai Bot commented Aug 15, 2026

Copy link
Copy Markdown

Warning

Review limit reached

You’ve reached a temporary PR review limit under our Fair Usage Limits Policy.

Your recent review volume is higher than typical usage, so adaptive limits are currently applied.

Next review available in: 17 minutes

Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available.
You're only billed for reviews past your plan's rate limits ($0.25/file).

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: ASSERTIVE

Plan: Pro

Run ID: 2cb9b12d-644d-4cc7-945e-5af5ef923582

📥 Commits

Reviewing files that changed from the base of the PR and between aca4583 and f888a64.

📒 Files selected for processing (9)
  • docs/specs/2026-07-30-mu-directory-design.md
  • src/MUI.Catalog/MsspLint.cs
  • src/MUI.Catalog/Persistence/ClaimService.cs
  • src/MUI.Web/Api/ApiResponse.cs
  • src/MUI.Web/Api/BadgeEndpoints.cs
  • src/MUI.Web/Components/Pages/Games.razor
  • tests/MUI.Catalog.Tests/MsspLintTests.cs
  • tests/MUI.Web.Tests/Api/BadgeApiTests.cs
  • tests/MUI.Web.Tests/PlainParityTests.cs

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

@HarryCordewener
HarryCordewener merged commit 67cf28e into main Aug 15, 2026
3 checks passed
@HarryCordewener
HarryCordewener deleted the fix/post-merge-cleanup branch August 15, 2026 19:56
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