Skip to content

chore(followups): post-batch cleanup from 2026-05-24 review - #592

Merged
junhoyeo merged 1 commit into
mainfrom
followups/post-batch-cleanup
May 25, 2026
Merged

chore(followups): post-batch cleanup from 2026-05-24 review#592
junhoyeo merged 1 commit into
mainfrom
followups/post-batch-cleanup

Conversation

@junhoyeo

@junhoyeo junhoyeo commented May 25, 2026

Copy link
Copy Markdown
Owner

Summary

Folds in the low-risk, behavior-preserving fold-in items captured during the 2026-05-24 batch review of #517 / #524 / #528 / #543 / #545 / #555 / #563 / #582 / #584 / #493.

Nothing here changes runtime behavior — they're test fixes, comments, README notes, and one stray-file deletion.

Change Source
Delete PR_BODY.md from repo root slipped in via #493
Add totalTokens/totalCost to EmbedContributionDay mocks in renderIsometric3DSvg.test.ts and renderProfileEmbedSvg.test.ts #584 added these fields to the type; both test files were already broken on main
Comment the userHasToggledSort URL-vs-click precedence #522
Comment the schemaVersion: 2 meaning in getSubmitDevice #517
README — Trae section: "vendor-reported" note for dollar_float #555
README — Subscription Usage: "vendor-reported" disclaimer #493

Why not bundle the rest?

The remaining followups from pr_followups_2026_05_24.md are either design discussions or investigations rather than concrete patches:

Each can become its own focused PR.

Test plan

  • bunx tsc --noEmit in packages/frontend returns zero errors (was 11 before this PR — all from the embed test mocks)
  • No conflict markers, no destructive deletions outside PR_BODY.md
  • CI green on this branch

Verification commands

cd packages/frontend && bunx tsc --noEmit  # 0 errors expected
git diff main..HEAD -- README.md           # 5 lines added across two sections

Summary by cubic

Post-batch cleanup to fix broken embed tests, add clarifying comments, add README disclaimers, and remove a stray file. No runtime behavior changes.

  • Bug Fixes

    • Update EmbedContributionDay mocks with totalTokens/totalCost so packages/frontend bunx tsc --noEmit is clean (renderIsometric3DSvg.test.ts, renderProfileEmbedSvg.test.ts).
  • Refactors

    • Add a comment documenting URL-vs-click sort precedence in LeaderboardClient.tsx (userHasToggledSort guard).
    • Document submission schemaVersion: 2 and the GREATEST() invariant in api/submit/route.ts.
    • README: add "vendor-reported" notes for Trae dollar_float and Subscription Usage quotas.
    • Delete stray PR_BODY.md from repo root.

Written for commit 36247b2. Summary will update on new commits. Review in cubic

Folds in the low-risk fold-in items captured in
`pr_followups_2026_05_24.md` after the 19-PR batch review/merge:

- Delete `PR_BODY.md` from the repo root (slipped in via #493).
- Fix `EmbedContributionDay` test mocks in `renderIsometric3DSvg.test.ts`
  and `renderProfileEmbedSvg.test.ts` — both were missing the
  `totalTokens`/`totalCost` fields that #584 added to the type, so
  `bunx tsc --noEmit` had been red on main since the embed-templates PR.
  Both files type-check clean now.
- Add a precedence-rule comment above `userHasToggledSort` in
  `LeaderboardClient.tsx` so the URL-`sortBy` → user-click handoff
  semantics survive future refactors. Each SortOption.onClick relies on
  flipping this ref; removing those lines silently regresses the URL
  param into a permanent override.
- Document the `schemaVersion: 2` value introduced in `getSubmitDevice`
  with a comment listing all three known schema versions and the
  `GREATEST()` invariant on the submissions row.
- Add a "vendor-reported" note to the README's Trae section explaining
  that cost figures come from Trae's `dollar_float` rather than
  tokscale's pricing engine, and a parallel disclaimer in the
  Subscription Usage section for the same reason.

Constraint: keep the PR scope to safe, behavior-preserving changes
Rejected: bundle env-overridable thresholds + ms/s normalization | both require design discussion and were left as separate followups
Rejected: add last-owner-cannot-be-demoted unit test | needs a non-trivial mocking surface for the role route; production code is already correct on main
Confidence: high
Scope-risk: narrow
Directive: do NOT remove the `userHasToggledSort.current = true;` lines in any SortOption.onClick — the comment now explains why
Not-tested: README rendering (markdown only)
@vercel

vercel Bot commented May 25, 2026

Copy link
Copy Markdown
Contributor

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
tokscale Ready Ready Preview, Comment May 25, 2026 1:54am

Request Review

@cubic-dev-ai cubic-dev-ai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

No issues found across 6 files

Re-trigger cubic

@junhoyeo
junhoyeo merged commit 8e66919 into main May 25, 2026
5 checks passed
@junhoyeo
junhoyeo deleted the followups/post-batch-cleanup branch May 25, 2026 02:16
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