test(app): cover the model-picker recordRecent wiring (follow-up #1347) - #1354
Conversation
…ctions Behavior-preserving: pulls the model picker's set/cycle out of the Local context's init into an exported, dependency-injected factory (mirroring createPromptBinding). The wiring was previously only reachable by mounting the whole context, so the recordRecent side-effect had no unit coverage.
Guards the recent-model default against a refactor that drops the wiring:
an explicit pick (set(item, { recent: true })) records to the server's
recent-model default; a plain set() and cycle() never do. The existing
ModelState/route/defaultModel tests would all stay green without this.
…ints The header told the upstream-TUI backstory and the recordRecent doc read like a PR description. Keep only what a maintainer needs: writes the recent list in state/model.json, ENOENT starts empty, other read failures skip to protect sibling state, atomic rename for the unlocked reader.
|
Warning Review limit reached
More reviews will be available in 48 minutes and 20 seconds. Learn how PR review limits work. Your organization has used up its prepaid credits, and credit purchases are no longer available. Enable the review add-on in the billing tab to keep reviews running — you're only billed for reviews past your plan's rate limits ($0.25/file). ⌛ How to resolve this issue?After more reviews become available, a review can be triggered using the 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 credits. 🚦 How do rate limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan refill rate. 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, the refill rate gradually slows as usage increases. The highest same-day bursts are limited more strictly. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (4)
✨ Finishing Touches🧪 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 |
github-actions
Bot
added
app
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
There was a problem hiding this comment.
Suggested priority: P2 (includes user-path files (packages/app/src/context/local.test.ts, packages/app/src/context/local.tsx)).
P1/P0 are reserved for maintainer confirmation. Please relabel manually if this is a release blocker, security issue, data-loss risk, or updater/runtime failure.
Astro-Han
added
the
task
The first cut extracted createModelActions and tested set(item,{recent:true})
directly. That bypassed the real wiring: deleting { recent: true } from the
model picker's onSelect left the test green, so it never guarded the regression
the PR is named for. It also added test-only production surface (an 8-param DI
factory) the component did not need — the picker already exposes a
ModelPickerState injection seam.
Drop createModelActions; restore Local's inline set/cycle. Move the picker's one
write path into model-picker-select.ts (selectModel) — a dependency-free sibling
the test imports without loading the component (Kobalte pulls solid-js's
client-only API, which throws under the server-condition test runtime; mirrors
sidebar-item-navigation.ts). ModelList.onSelect now calls selectModel, and the
unit test asserts an explicit pick passes { recent: true }. Verified by dropping
the flag and watching the test go red.
The programmatic-set / cycle "do not record recent" assertions are dropped with
the factory: they have no light real-path seam (only a full LocalProvider mount,
the heavy e2e an earlier review vetoed). The picker call site is the high-value
guard.
Claude-Session: https://claude.ai/code/session_01PWbeoRm3mmxYxKae2Kj6Z7
1 participant
Follow-up to the #1347 review (recent-model default). Two items:
P2 — wiring test (real coverage gap).
model.set(item, { recent: true })is the only path that callsprovider.recordRecent; a refactor that drops{ recent: true }at the picker call site, or deletes the client call, would leave every existing test green (theModelState, route, anddefaultModeltests all exercise the server side, not this client wiring).The picker's single write path is extracted into a dependency-free
selectModel()inmodel-picker-select.ts, andmodel-picker.tsx'sModelList.onSelectcalls it. The extraction is the only way to unit-test this call site: importing the component itself pulls@kobalte/core, which throwsClient-only API called on the server sideunder the bun server-condition test runtime, so the real picker interaction is only reachable via Playwright e2e.model-picker-select.test.tsasserts the load-bearing invariant — an explicit pick and clearing the selection both pass{ recent: true }. The red/green gate was confirmed: dropping the flag turns the test red.Scope note: the negative cases (a plain programmatic
model.set(item)andmodel.cycle()must not record) are intentionally not asserted here. There is no light real-path seam for them —set/cyclelive on theLocalProvider, and exercising them would require mounting the full context, the heavy e2e the #1347 review vetoed. An earlier attempt extracted them into acreateModelActionsfactory; that added production surface for test-only reasons (YAGNI) and was reverted —local.tsx/local.test.tsare back at the merge base.P3 — comment trim.
model-state.tshad an upstream-TUI backstory header and a PR-description-stylerecordRecentdoc. Trimmed to the load-bearing constraints (writes the recent list instate/model.json; ENOENT starts empty; other read failures skip to protect sibling state; atomic rename for the unlocked reader).Verification
bun test src/components/prompt-input/model-picker-select.test.ts src/context/local.test.ts— 8 pass (2 new picker-select + 6 unchanged local){ recent: true }fromselectModelturns the test redbun run typecheck— app + opencode clean (CI)