Skip to content

feat: buddy color progression (species x rarity x XP)#126

Open
Ldaco wants to merge 24 commits into
masterfrom
feature/color-progression
Open

feat: buddy color progression (species x rarity x XP)#126
Ldaco wants to merge 24 commits into
masterfrom
feature/color-progression

Conversation

@Ldaco
Copy link
Copy Markdown
Collaborator

@Ldaco Ldaco commented May 13, 2026

Summary

Replaces the single hard-coded MAGENTA sprite color with a three-input gradient driven by (species, rarity, totalXp), so every buddy reads as a distinct, evolving color identity.

  • New module src/lib/color.ts — 21 species palettes (84 RGB anchors) + 5 rarity metal tiers (10 anchors), saturation tint, bold weight at Rare+, and a four-tier terminal capability cascade (truecolor / 256-color / 16-color / NO_COLOR).
  • Public API: colorFor(species, rarity, totalXp, caps?) returns the appropriate ANSI escape (or empty string for NO_COLOR).
  • Integrations:
    • statusline-wrapper.ts normal sprite mode
    • statusline-wrapper.ts bubble sprite mode
    • card.ts renderCard()
    • share.ts renderShareHtml() — Puppeteer/PNG output uses inline rgb() + font-weight: bold for Rare+
  • Design: species color is the dominant identity at low levels; from Lv ~30-40 the buddy bridges into the rarity's metal anchor (Common -> Iron, Uncommon -> Copper, Rare -> Gold, Epic -> Diamond, Legendary -> Aurum). Saturation tint per rarity (0.85x for Common up to 1.20x for Legendary) reinforces the tier feel.

Spec: docs/superpowers/specs/2026-05-12-buddy-color-progression-design.md
Plan: docs/superpowers/plans/2026-05-12-buddy-color-progression.md

Test plan

  • 79 unit tests in src/__tests__/color.test.ts (every exported function covered)
  • 15-fixture snapshot contract in src/__tests__/color-snapshot.test.ts
  • Card integration tests in src/__tests__/card-color.test.ts (3 tests)
  • Share/HTML integration tests in src/__tests__/share-color.test.ts (7 tests)
  • Full suite: 829 pass, 2 pre-existing Penguin failures unrelated to this branch
  • Manual verification across capability tiers (NO_COLOR / truecolor / 256-color / 16-color) all produce the expected output paths
  • PNG share renders verified visually for 6 species/rarity/level combinations including Data Drake Common Lv 50 -> iron-gray and Legendary Lv 50 -> aurum

Notes

  • The 2 pre-existing Penguin sprite test failures (`animation-stability.test.ts`, `species.test.ts`) were present on master before this branch and are unrelated to color work.
  • Branch is currently behind origin/master by 12 commits (graph CLI + installer fixes); no expected conflicts (disjoint files), happy to rebase if maintainer prefers.

Ldaco and others added 22 commits May 12, 2026 19:32
Brainstormed three-input color model (species × rarity × XP) that replaces
the current single-MAGENTA sprite color with a continuous gradient through
21 species palettes (84 colors) and 5 rarity-metal tiers (10 colors).
Rarity uses a tier-break ladder (Iron/Copper/Gold/Diamond/Aurum) so "rare"
is visibly rare. Saturation tint and bold weight at Rare+ make rarity
readable from Lv 1.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Spec self-review pass: tighten three ambiguities.
- Saturation tint applies uniformly to species AND metal segments (intentional).
- Buddy's user-given name stays CYAN; only sprite-art colors change.
- Acknowledge Lv 30-40 bridge can look muddy for some (species, rarity)
  combos; flag for per-species tuning post-launch.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
18-task plan covering: new src/lib/color.ts module (palettes, capability
detection, RGB math), integration into statusline-wrapper.ts and card.ts,
unit + snapshot + integration tests, manual verification across truecolor /
256-color / 16-color / NO_COLOR modes.

Each task is TDD-disciplined (test → fail → impl → pass → commit) with
exact file paths, commands, and code shown inline.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Remove the design spec and implementation plan files that were used as
working artifacts during development. They've served their purpose and
the code is now the source of truth.

Also strip the now-stale spec reference comment from src/lib/color.ts.
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Introduces a new buddy sprite color system that evolves based on (species, rarity, totalXp) and wires it into terminal rendering (statusline + card) and share HTML output.

Changes:

  • Added src/lib/color.ts with palette/rarity anchoring, XP-driven interpolation, and terminal capability detection (truecolor/256/16/NO_COLOR).
  • Updated statusline wrapper and card rendering to use colorFor(...) instead of a single hard-coded sprite color.
  • Updated share HTML rendering to style the sprite placeholder with an inline rgb(...) color and bold weight for Rare+; added unit + snapshot + integration tests.

Reviewed changes

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

Show a summary per file
File Description
src/lib/color.ts New color computation + terminal capability detection + colorFor public API.
src/statusline-wrapper.ts Uses colorFor for sprite art in normal + bubble modes.
src/lib/card.ts Wraps sprite art lines in colorFor(...) escape codes.
src/lib/share.ts Applies computed RGB + bold weight to the share sprite placeholder via inline style.
src/tests/color.test.ts Unit tests for constants/helpers/capability detection and colorFor.
src/tests/color-snapshot.test.ts Snapshot fixtures to lock down ANSI escape output.
src/tests/card-color.test.ts Card integration tests for ANSI color injection.
src/tests/share-color.test.ts Share HTML integration tests for inline RGB/bold behavior.
src/tests/snapshots/color-snapshot.test.ts.snap Snapshot outputs for the fixture contract.
Comments suppressed due to low confidence (1)

src/statusline-wrapper.ts:285

  • Same issue as bubble mode: buddy.rarity comes from JSON and can be missing/invalid, but it’s passed directly into colorFor(). If rarity is undefined this will throw and break the statusline rendering. Please default/validate rarity before calling colorFor (consistent with the earlier buddy.rarity || 'common' fallback).
        const artWidth = Math.max(...asciiLines.map((l: string) => l.length));
        const spriteColor = colorFor(buddy.species, buddy.rarity, buddy.xp);
        for (let i = 0; i < asciiLines.length; i++) {
          const artPart = `${spriteColor}${(asciiLines[i] || "").padEnd(artWidth)}${RESET}`;
          if (i === 0) {

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread src/statusline-wrapper.ts
Comment thread src/lib/card.ts Outdated
Comment thread src/lib/color.ts Outdated
Three robustness fixes from the PR review:

1. colorFor respects caps.ansi16 — previously fell back to ANSI-16 escapes
   even when the flag was false, making TerminalCapabilities lie. Now
   returns '' if no capability flag is set.

2. card.ts skips RESET when spriteColor is empty — NO_COLOR mode no longer
   leaks a stray \x1b[0m on every sprite line.

3. statusline-wrapper.ts defaults buddy.rarity to 'common' and buddy.xp to
   0 at both colorFor call sites — buddy state is parsed from JSON and
   rarity can be missing/invalid; without the default, RARITY_METALS lookup
   would throw. Mirrors the existing buddy.rarity || 'common' pattern on
   line 86. Also makes the RESET conditional in both bubble and normal
   modes, same as card.ts.
@Ldaco Ldaco self-assigned this May 13, 2026
@Ldaco Ldaco added the enhancement New feature or request label May 13, 2026
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

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

Comments suppressed due to low confidence (1)

src/statusline-wrapper.ts:286

  • Same runtime-safety issue as bubble mode: buddy.rarity || 'common' won’t protect against an invalid-but-truthy rarity string from buddy-status.json. If colorFor() throws here, the statusline wrapper will output nothing. Recommend validating rarity before calling colorFor() and falling back to 'common'/no color for unknown values.

        const artWidth = Math.max(...asciiLines.map((l: string) => l.length));
        const spriteColor = colorFor(buddy.species, buddy.rarity || 'common', buddy.xp ?? 0);
        const spriteReset = spriteColor ? RESET : '';
        for (let i = 0; i < asciiLines.length; i++) {
          const artPart = `${spriteColor}${(asciiLines[i] || "").padEnd(artWidth)}${spriteReset}`;

Comment thread src/statusline-wrapper.ts
Comment thread src/lib/color.ts
Comment thread src/lib/card.ts Outdated
Comment on lines +52 to +53
const inner = padded.slice(prefix.length, padded.length - suffix.length);
return `${prefix}${spriteColor}${inner}${spriteReset}${suffix}`;
Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Renamed to artInner in 6cf9891.

Comment thread src/__tests__/card-color.test.ts
Four findings:

1. computeRGB now falls back to common metals/saturation for invalid
   rarities (e.g., stale state file post schema migration). Better to
   render a muted sprite than crash the whole statusline. This defends
   both the buddy.rarity || 'common' seam in statusline-wrapper and
   any future JSON callers — TS still enforces typed callers.

2. card.ts renames the shadowed `inner` to `artInner` to avoid
   collision with the outer card-inner-width binding.

3. card-color.test.ts adds a precondition that expectedColor is not
   empty, so the toContain assertion isn't vacuous under NO_COLOR.

4. New unit test covers computeRGB's invalid-rarity fallback path,
   asserting it produces the same RGB as the common-rarity baseline.
@fiorastudio
Copy link
Copy Markdown
Owner

I reviewed PR #126 locally with your “impact on existing users” concern in mind. I also kicked off a parallel Codex review, but it timed out before returning, so
this is my direct review only.

Bottom line

  • I think feat: buddy color progression (species x rarity x XP) #126 is low-to-moderate risk for existing users.
  • It appears to be a presentation-only change: buddy sprite art gets colorized in terminal card output, share snapshots, and the statusline.
  • I do not see install, storage, config, migration, or API changes that would break existing users.
  • My main concern is terminal compatibility / readability, not data loss or upgrade breakage.

What changes for existing users

  • renderCard() now wraps sprite lines with a dynamic ANSI color from colorFor(...) in src/lib/card.ts:43.
  • Statusline sprite/bubble art also switches from a fixed magenta to species/rarity/xp-based color in src/statusline-wrapper.ts:167 and src/statusline-
    wrapper.ts:280.
  • Share HTML uses computed RGB and font-weight for the sprite in src/lib/share.ts:16 and src/lib/share.ts:201.
  • New color system lives in src/lib/color.ts:1.

So existing users should mostly experience:

  • same buddy features
  • same commands
  • same data
  • same cards/statusline/share output structure
  • different colors

Existing-user impact assessment

  • Safe areas
    • No DB schema change
    • No new config requirement
    • No tool/API signature change
    • No installer/uninstaller changes
    • No change to companion state or saved data model
  • Risk areas
    • More ANSI escape sequences in card/statusline output
    • Potential rendering oddities in terminals with weak color support
    • Potential snapshot/automation diffs if anyone parses terminal output literally

My main concern

  • The PR changes long-standing terminal output from fixed color behavior to computed per-buddy color behavior, but I haven’t yet seen explicit guarantees about
    fallback behavior for:
    • no color
    • ANSI-16 only
    • ANSI-256 only
    • unsupported / quirky terminals
  • If colorFor() handles those correctly, risk stays low.
  • If it assumes richer color capability too aggressively, existing users could get ugly or inconsistent output rather than broken functionality.

What I’d specifically ask before merge

  1. Verify fallback behavior
    • Confirm NO_COLOR still disables sprite color cleanly.
    • Confirm ANSI-16/ANSI-256 fallback is readable and not noisy.
    • Confirm plain terminals don’t get malformed escape spam.
  2. Call out user-visible change in PR
    • Existing users will notice a visual change immediately in statusline/card/share.
    • That should be explicit in the PR description.
  3. Consider rollout sensitivity
    • If you’re worried about existing-user surprise, this is a good candidate for:
      • a feature flag
      • or at least a conservative fallback path

Merge recommendation

  • Probably okay to merge if terminal fallback behavior is solid.
  • If you want to be extra careful for current users, request one final check:
    • “Please verify NO_COLOR, ANSI-16, ANSI-256, and truecolor behavior before merge.”

Suggested review comment

  • “This looks mostly presentation-only, so I’m not too worried about breaking existing users functionally. My main concern is terminal compatibility/readability,
    since this changes long-standing card/statusline output from fixed colors to computed ANSI colors. Before merge, can we confirm fallback behavior is solid for
    NO_COLOR, ANSI-16, ANSI-256, and truecolor terminals, and call out the visual change explicitly in the PR description?”

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants