Skip to content

fix(server): show native provider labels for Pi models - #14941

Closed
akemmanuel wants to merge 1 commit into
pingdotgg:mainfrom
akemmanuel:fix/pi-model-source-labels
Closed

akemmanuel wants to merge 1 commit into
pingdotgg:mainfrom
akemmanuel:fix/pi-model-source-labels

Conversation

@akemmanuel

Copy link
Copy Markdown

Problem

Pi can expose identically named models through different native providers. In the model picker, these rows all show only Pi, so users cannot tell which source they are selecting.

parseDiscoveredModels already reads the native provider ID and includes it in the model slug, but omits subProvider from the discovery snapshot. The existing picker preserves and displays that field as Pi · <source> when present.

Fix

Preserve the native provider ID in subProvider. Add a regression test that exercises discovery through a mocked Pi RPC subprocess with two identically named models from distinct providers.

Model slugs, routing, favorites, and client layout are unchanged. This qualifies as the small, focused obvious-bug exception in CONTRIBUTING.md: it restores metadata used by the existing picker rather than introducing new product behavior. No startup, installation, or service changes are included.

Verification

  • Before the fix, the new regression test failed because both discovered models had subProvider: undefined; their distinct slugs were already correct.
  • After the fix: bun x --no-install vp test run apps/server/src/provider/Layers/PiProvider.test.ts apps/web/src/modelSelection.test.ts apps/web/src/components/chat/ModelPickerContent.test.ts — 3 files, 62 tests passed.
  • Targeted vp lint for the two changed files and git diff --check passed.
  • On two Linux hosts using Node 24.21.0 and Pi 1.0.0, real Pi discovery returned ready snapshots with native provider labels populated for GPT-6 Luna models.
  • No post-fix browser automation, desktop, or mobile verification was performed. This changes only server discovery metadata; the existing contract and picker support subProvider already.

Model: gpt-6.1-sol. Harness: Pi.

@github-actions github-actions Bot added vouch:unvouched PR author is not yet trusted in the VOUCHED list. size:XS 0-9 changed lines (additions + deletions). labels Oct 2, 2026
@macroscopeapp

macroscopeapp Bot commented Oct 3, 2026

Copy link
Copy Markdown
Contributor

Approvability

Verdict: Approved at a8b5a51

Macroscope's review found this PR approvable — This is a small, self-contained server bug fix that populates an existing optional provider-label field for discovered Pi models. Existing UI support and a focused regression test account for the runtime impact without changing model selection, routing, defaults, or infrastructure.

You can add or adjust custom eligibility rules. Learn more.

@coderabbitai

coderabbitai Bot commented Oct 3, 2026

Copy link
Copy Markdown

Review in Change Stack →

Navigate logical layers of code changes, visualize relationships, and explore their blast radius.

🧰 Additional context used
📚 Code guidelines (1)
docs/internals/effect-services.md — configured

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration
  • Configuration used: Repository: pingdotgg/t3code/.coderabbit.yaml
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: 7e3bd1e1-8ed5-4d64-ba35-3024d7088b49
📥 Commits

Reviewing files that changed from the base of the PR and between 7ff2eab and a8b5a51.

📒 Files selected for processing (2)
  • apps/server/src/provider/Layers/PiProvider.test.ts
  • apps/server/src/provider/Layers/PiProvider.ts

Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 9 remain after this review.


📝 Walkthrough

Walkthrough

Pi model discovery now stores each model’s provider name in subProvider. A new test checks that same-named models from different providers retain distinct provider-qualified slugs and matching subProvider values.

Changes

Pi model provider identity

Layer / File(s) Summary
Preserve provider identity during discovery
apps/server/src/provider/Layers/PiProvider.ts, apps/server/src/provider/Layers/PiProvider.test.ts
Discovered model records now set subProvider to the model’s provider. The test fixture returns two same-named models from different providers and checks their slugs and subProvider values.

Priority: ⬇️ Low

Estimated code review effort: 2 (Simple) | ~8 minutes

Change: Bug fix

Suggested reviewers: juliusmarminge

Merge Risk: ⚪ Minimal · up to a8b5a

Pi models in the picker will now show their native provider label. Model slugs and routing are unchanged, and no merge-blocking risk was found.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 3 functions across 2 files. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title is concise, uses the conventional commit format, and clearly describes the primary change: showing native provider labels for Pi models.
Description check ✅ Passed The description covers the problem, fix, scope rationale, regression test, verification results, and known verification limits. It uses a "Fix" heading instead of the template's "Change" heading and d…
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.
  • Fix all pre-merge checks with AI
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create a new PR
  • Autopilot · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Autopilot is currently an internal CodeRabbit preview.


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

aliceisjustplaying added a commit to aliceisjustplaying/t3code that referenced this pull request Oct 6, 2026
aliceisjustplaying added a commit to aliceisjustplaying/t3code that referenced this pull request Oct 6, 2026
pingdotgg#15529's new test parses and writes JSON with JSON.parse/stringify inside
Effect.gen, which the effect language service rejects (preferSchemaOverJson),
so the server typecheck failed on fork-stack. `ours` does not have that code to
change, so build-stack.sh now commits fixups/<pr>.patch once every PR is
merged; applying it earlier changes pingdotgg#14941's conflict with pingdotgg#15079 and its
recorded resolution no longer matches. The patch uses the Schema helpers pingdotgg#15079
adds to the same file.
aliceisjustplaying added a commit to aliceisjustplaying/t3code that referenced this pull request Oct 6, 2026
aliceisjustplaying added a commit to aliceisjustplaying/t3code that referenced this pull request Oct 6, 2026
pingdotgg#15529's new test parses and writes JSON with JSON.parse/stringify inside
Effect.gen, which the effect language service rejects (preferSchemaOverJson),
so the server typecheck failed on fork-stack. `ours` does not have that code to
change, so build-stack.sh now commits fixups/<pr>.patch once every PR is
merged; applying it earlier changes pingdotgg#14941's conflict with pingdotgg#15079 and its
recorded resolution no longer matches. The patch uses the Schema helpers pingdotgg#15079
adds to the same file.
@juliusmarminge

Copy link
Copy Markdown
Member

Note

Grok responding on behalf of Julius.

Thanks for this! The same fix (preserving the native provider in subProvider for Pi-discovered models) just landed on main in #16661, so I'm closing this as superseded. If you think your regression test still adds coverage on top of current main, feel free to open a small follow-up PR with just the test.

aliceisjustplaying added a commit to aliceisjustplaying/t3code that referenced this pull request Oct 7, 2026
pingdotgg#15529's new test parses and writes JSON with JSON.parse/stringify inside
Effect.gen, which the effect language service rejects (preferSchemaOverJson),
so the server typecheck failed on fork-stack. `ours` does not have that code to
change, so build-stack.sh now commits fixups/<pr>.patch once every PR is
merged; applying it earlier changes pingdotgg#14941's conflict with pingdotgg#15079 and its
recorded resolution no longer matches. The patch uses the Schema helpers pingdotgg#15079
adds to the same file.
aliceisjustplaying added a commit to aliceisjustplaying/t3code that referenced this pull request Oct 7, 2026
pingdotgg#15529's new test parses and writes JSON with JSON.parse/stringify inside
Effect.gen, which the effect language service rejects (preferSchemaOverJson),
so the server typecheck failed on fork-stack. `ours` does not have that code to
change, so build-stack.sh now commits fixups/<pr>.patch once every PR is
merged; applying it earlier changes pingdotgg#14941's conflict with pingdotgg#15079 and its
recorded resolution no longer matches. The patch uses the Schema helpers pingdotgg#15079
adds to the same file.
aliceisjustplaying added a commit to aliceisjustplaying/t3code that referenced this pull request Oct 7, 2026
pingdotgg#15529's new test parses and writes JSON with JSON.parse/stringify inside
Effect.gen, which the effect language service rejects (preferSchemaOverJson),
so the server typecheck failed on fork-stack. `ours` does not have that code to
change, so build-stack.sh now commits fixups/<pr>.patch once every PR is
merged; applying it earlier changes pingdotgg#14941's conflict with pingdotgg#15079 and its
recorded resolution no longer matches. The patch uses the Schema helpers pingdotgg#15079
adds to the same file.
aliceisjustplaying added a commit to aliceisjustplaying/t3code that referenced this pull request Oct 8, 2026
pingdotgg#15529's new test parses and writes JSON with JSON.parse/stringify inside
Effect.gen, which the effect language service rejects (preferSchemaOverJson),
so the server typecheck failed on fork-stack. `ours` does not have that code to
change, so build-stack.sh now commits fixups/<pr>.patch once every PR is
merged; applying it earlier changes pingdotgg#14941's conflict with pingdotgg#15079 and its
recorded resolution no longer matches. The patch uses the Schema helpers pingdotgg#15079
adds to the same file.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size:XS 0-9 changed lines (additions + deletions). vouch:unvouched PR author is not yet trusted in the VOUCHED list.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants