Extract ActivityCard model colors to lib/constants/model-colors.ts - #143
Conversation
Pure refactor per the ROADMAP "Model colors outside design system" item — same colors, same match order, no behavior change.
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Warning Review limit reachedNext included review available in 54 minutes. View limit detailsLimit details: You’ve used the included review currently available. You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. Review configuration: ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughModel chip color resolution moved from ChangesModel Color Extraction
Estimated code review effort: 2 (Simple) | ~10 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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. Comment |
Resolves the docs/CHANGELOG.md conflict by folding the model-colors entry into the existing Unreleased/Changed section rather than opening a second one. Also fixes a real bug in the extracted code: hashString returns a signed 32-bit int, so a name that hashed negative indexed off the front of the fallback palette and gave the chip an undefined backgroundColor — about half of all unknown names, hidden from TypeScript by a non-null assertion. It only became reachable in production when collection started accepting every ccusage source, so names like kimi-k2 and deepseek-v3 now hit it. modelColor and its hash move into lib/constants/model-colors.ts so the module owns the whole name-to-colour decision instead of exporting raw tables for callers to recombine, and so it is unit-testable without mounting the card. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
apps/web/lib/constants/model-colors.ts (1)
14-23: 📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick winReplace raw hex values with
globals.csstheme tokens.These model-chip colors bypass the shared theme. Define or use the required theme colors, then reference their CSS variables from this constant module.
As per coding guidelines,
apps/web/**/*.{css,tsx,ts}must use colors from theglobals.css@themeblock.🤖 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 `@apps/web/lib/constants/model-colors.ts` around lines 14 - 23, Update MODEL_COLOR_PATTERNS to replace every raw hex color with the corresponding CSS variables defined in the globals.css `@theme` block. Add any missing theme tokens there first, then reference those variables from this constant while preserving the existing model-pattern mappings and order.Source: Coding guidelines
🤖 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/CHANGELOG.md`:
- Line 7: Update the fallback palette indexing in modelColor() within
model-colors.ts to normalize the signed hash modulo result into a non-negative
index before accessing MODEL_COLOR_FALLBACK_PALETTE. Preserve valid palette
selection for all model names, then retain the changelog entry only once this
implementation fix is applied.
---
Outside diff comments:
In `@apps/web/lib/constants/model-colors.ts`:
- Around line 14-23: Update MODEL_COLOR_PATTERNS to replace every raw hex color
with the corresponding CSS variables defined in the globals.css `@theme` block.
Add any missing theme tokens there first, then reference those variables from
this constant while preserving the existing model-pattern mappings and order.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 666aeaaa-c725-4a7b-8a62-d798ff6bf898
📒 Files selected for processing (5)
apps/web/__tests__/unit/model-colors.test.tsapps/web/components/app/feed/ActivityCard.tsxapps/web/lib/constants/model-colors.tsdocs/CHANGELOG.mddocs/ROADMAP.md
💤 Files with no reviewable changes (1)
- docs/ROADMAP.md
🚧 Files skipped from review as they are similar to previous changes (1)
- apps/web/components/app/feed/ActivityCard.tsx
|
|
||
| ### Fixed | ||
|
|
||
| - **Model chips no longer render colourless for unknown models.** The fallback palette was indexed with a signed 32-bit hash, so any model name that hashed negative indexed off the front of the array and handed the chip an `undefined` background — about half of all names, hidden from TypeScript by a non-null assertion. Now that collection accepts every ccusage source, real names like `kimi-k2` and `deepseek-v3` hit this. The hash is folded to a positive index and the mapping is covered by unit tests. |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Do not document the fallback-index fix until the implementation normalizes negative indexes.
apps/web/lib/constants/model-colors.ts still uses hashString(name) % MODEL_COLOR_FALLBACK_PALETTE.length. A negative signed hash produces a negative array property, so modelColor() can still return undefined. ActivityCard then receives an invalid backgroundColor.
Normalize the modulo result before updating this changelog entry.
Proposed fix
- const index = hashString(name) % MODEL_COLOR_FALLBACK_PALETTE.length;
+ const hash = hashString(name);
+ const index =
+ ((hash % MODEL_COLOR_FALLBACK_PALETTE.length) +
+ MODEL_COLOR_FALLBACK_PALETTE.length) %
+ MODEL_COLOR_FALLBACK_PALETTE.length;🤖 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/CHANGELOG.md` at line 7, Update the fallback palette indexing in
modelColor() within model-colors.ts to normalize the signed hash modulo result
into a non-negative index before accessing MODEL_COLOR_FALLBACK_PALETTE.
Preserve valid palette selection for all model names, then retain the changelog
entry only once this implementation fix is applied.
|
Merged The fallback palette was indexed with a signed hash. This is reachable in production now, not theoretically. It only bit models that miss every known-family pattern, which used to mean almost nothing since we collected Claude and Codex — but collection takes every ccusage source now, so Gemini, Kimi, DeepSeek and Qwen names all land in the fallback path and roughly half of them come out colourless. Shape change from the original PR. It exported the two tables and left Tests: Conflict was Typecheck 0, lint 0, 5/5 new tests pass. |
|
CI red here is not this PR. Every open PR is failing the same single assertion: That test pins hardcoded LiteLLM dollar amounts for the GPT-5.6 family, and LiteLLM repriced #150 replaces the pinned rates with the invariant (every model resolves to a non-zero price, day total equals the sum of the breakdown). Merge that first and this should go green on a re-run. |
…rs-constants # Conflicts: # docs/CHANGELOG.md
2 participants
Summary
Roadmap item ("UX Polish" → "Model colors outside design system"):
ActivityCard.tsxused hardcoded hex colors inline for model chips. Extracted them toapps/web/lib/constants/model-colors.tsasMODEL_COLOR_PATTERNS(ordered pattern→color list) andMODEL_COLOR_FALLBACK_PALETTE(hash-based fallback palette), matching the existinglib/constants/regions.tsconvention.Pure refactor — same colors, same match order, no behavior change.
Changes
apps/web/lib/constants/model-colors.ts— new file with the extracted constants.apps/web/components/app/feed/ActivityCard.tsx—modelColor()now iteratesMODEL_COLOR_PATTERNSand falls back toMODEL_COLOR_FALLBACK_PALETTE.docs/CHANGELOG.md— entry under Unreleased/Changed.docs/ROADMAP.md— removed the now-completed item.Test plan
bun run lint— cleanbun run typecheck— cleanbun run test— 646/646 web tests pass, 199/199 CLI tests pass (unaffected)Generated by Claude Code
Summary by CodeRabbit