Skip to content

feat(career): practice invitations — closest stamps + bring-these-up#953

Merged
byrongamatos merged 2 commits into
mainfrom
feat/career-invitations
Jul 13, 2026
Merged

feat(career): practice invitations — closest stamps + bring-these-up#953
byrongamatos merged 2 commits into
mainfrom
feat/career-invitations

Conversation

@byrongamatos

@byrongamatos byrongamatos commented Jul 13, 2026

Copy link
Copy Markdown
Contributor

Career v3, workstream 1. The passport now points at the practice that pays:

  • Stubs carry next_star_at (via a new shared _star_progress() — the one place the ascending-thresholds assumption lives, now used by _stars() too), and each in-progress passport exposes nearest: the top 3 non-qualifying songs ranked by distance to the qualifying bar (the badge ask — a 0★ song 1% from its next star is not closer to a ★★ badge than a 1★ song 5% from the bar).
  • "Closest stamps" strip above the shelf: the in-progress passports nearest to minting, each row voicing the ask through one shared ppAskHTML() phrase — N more ★★ songs (with the nearest title at a floored %, so 74.9% can never display as the already-met "75%") or the blocking Virtuoso drill. Rows open the passport. The passport book's invite line uses the same shared phrase, so strip and book can never contradict.
  • "Bring these up" list on the stubs page of in-progress passports (best % → bar %); earned pages stay memorabilia.
  • Invitation-voiced throughout — no meters, no completion pressure.

Review notes

Self-review (Codex is quota-locked until Jul 19; the internal review pass substituted): caught and fixed round-up-past-the-bar display, two unguarded derefs that could blank the passports tab on a malformed payload, next-star vs qualifying-bar ranking mismatch, the duplicated blocker phrasing, and a dead guard.

Testing

Bar-ranking vs next-star test, nearest ordering/exclusions/cap, full career suites (31 pytest + 9 vm), full JS glob 1171 green.

🤖 Generated with Claude Code

Summary by CodeRabbit

  • New Features

    • Added a “Closest stamps” strip to highlight genres nearest to earning a passport stamp.
    • Added “Bring these up” recommendations with up to 3 songs near the qualifying bar, including accuracy and progress details.
    • Updated the passports API response to include nearest recommendations per genre.
  • Style

    • Added “closest”/“nearest” stamp-like UI styling, including typography, hover treatment, and divider visuals.
  • Tests

    • Added coverage for nearest recommendation ordering, exclusions, result limiting, and bar-at values.

Career v3, WS1. The passport now points at the practice that pays:

- Stubs carry next_star_at (the same primitive _stars() uses) and each
  passport exposes `nearest`: the top 3 non-qualifying songs by distance
  to their next star, in the worklist order.
- "Closest stamps" strip above the shelf: the in-progress graded
  passports nearest to minting, each row naming the ask — N more songs
  (with the nearest title + best %) or the blocking Virtuoso drill.
  Rows open the passport.
- "Bring these up" list on the stubs page of in-progress passports;
  earned pages stay memorabilia (no homework on a won badge).
- Invitation-voiced throughout: no meters, no completion pressure.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@coderabbitai

coderabbitai Bot commented Jul 13, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: 1841e359-c066-45ff-b99d-5d59ce5bb932

📥 Commits

Reviewing files that changed from the base of the PR and between 486773e and c4b9acd.

📒 Files selected for processing (1)
  • tests/plugins/career/test_passports.py
🚧 Files skipped from review as they are similar to previous changes (1)
  • tests/plugins/career/test_passports.py

📝 Walkthrough

Walkthrough

Career passports now calculate nearest non-qualifying songs, expose them through the passport API, and render closest-stamp invitations plus nearest-song practice sections in the UI.

Changes

Career passport invitations

Layer / File(s) Summary
Passport star progress and nearest results
plugins/career/routes.py, tests/plugins/career/test_passports.py
Star progress is centralized, passport song stubs include next-star thresholds, and passport responses return up to three nearest non-qualifying songs with coverage for ordering and qualifying-bar selection.
Closest and nearest invitation rendering
plugins/career/screen.html, plugins/career/screen.js
The passports panel gains a closest-invitations mount, shared invite text generation, closest-stamp rendering, and nearest-song sections within in-progress passport books.
Invitation presentation styles
plugins/career/assets/career.css
Closest and nearest invitation containers, headers, rows, typography, hover styling, and dividers receive dedicated CSS rules.

Estimated code review effort: 3 (Moderate) | ~20 minutes

Sequence Diagram(s)

sequenceDiagram
  participant PassportUI
  participant CareerAPI
  participant PassportView
  PassportUI->>CareerAPI: request passports
  CareerAPI->>PassportView: build passport data
  PassportView-->>CareerAPI: return nearest songs and thresholds
  CareerAPI-->>PassportUI: return passport JSON
  PassportUI->>PassportUI: render closest stamps and nearest songs
Loading

Possibly related PRs

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Description check ⚠️ Warning The description summarizes the change and testing, but it omits the required template sections like What, feedpak surface, and the checklist. Add the required template headings, include the feedpak-surface decision if applicable, and complete the checklist items for changelog, tests, and DCO sign-off.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title is concise and accurately summarizes the career practice-invitation and closest/bring-these-up UI work.
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 docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch feat/career-invitations

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

@coderabbitai coderabbitai 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.

Actionable comments posted: 1

🤖 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 `@tests/plugins/career/test_passports.py`:
- Around line 251-256: Update the comments above the feedpak fixtures to
describe ranking by the qualifying bar rather than distance to the next-star
cutoff. Keep the existing assertions and fixture values unchanged, and ensure
the qualifying-song exclusion and three-song cap remain accurately documented.
🪄 Autofix (Beta)

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: 91cbef9c-fd34-48a5-a024-02702d232c12

📥 Commits

Reviewing files that changed from the base of the PR and between 6cc0312 and 486773e.

📒 Files selected for processing (5)
  • plugins/career/assets/career.css
  • plugins/career/routes.py
  • plugins/career/screen.html
  • plugins/career/screen.js
  • tests/plugins/career/test_passports.py

Comment thread tests/plugins/career/test_passports.py Outdated
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@byrongamatos
byrongamatos merged commit 831117f into main Jul 13, 2026
6 checks passed
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