Repository navigation
Support Pi in LLM gateway setup - #6818
Conversation
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #6818 +/- ##
==========================================
+ Coverage 79.64% 79.66% +0.01%
==========================================
Files 816 817 +1
Lines 83168 83411 +243
==========================================
+ Hits 66242 66446 +204
- Misses 16921 16960 +39
Partials 5 5 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
🟡 Changes recommended
Lazy Anthropic models are misclassified, and valid Pi configurations or custom agent directories are not handled.
3 open findings
What changed in this PR
Adds Pi support to LLM gateway setup, including dual-protocol model discovery and managed provider configuration.
Changes:
- Registers Pi and safely manages ToolHive providers.
- Discovers OpenAI and paginated Anthropic model metadata.
- Adds unit tests and CLI documentation.
| File | Description |
|---|---|
pkg/llmgateway/config.go |
Adds Pi mode and model metadata fields. |
pkg/llm/setup.go |
Implements dual-protocol model discovery. |
pkg/llm/setup_test.go |
Tests discovery, pagination, and setup. |
pkg/client/llm_gateway.go |
Routes Pi configuration and teardown. |
pkg/client/llm_gateway_pi.go |
Manages Pi provider configuration. |
pkg/client/llm_gateway_pi_test.go |
Tests Pi setup and teardown. |
pkg/client/config.go |
Registers the Pi client integration. |
cmd/thv/app/llm.go |
Documents Pi CLI behavior. |
docs/cli/thv_llm_setup.md |
Regenerates setup documentation. |
🧠 Review effort: Balanced
💡 Configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
aponcedeleonch
left a comment
There was a problem hiding this comment.
Nice work on this. I checked the setup against Pi's docs and source (pi.dev and earendil-works/pi). The !thv llm token apiKey, authHeader, the PI_CODING_AGENT_DIR handling and the JSONC-preserving edits all line up.
Before merge, I'd want the one-hour prompt cache and the hardcoded /anthropic prefix addressed. The rest are smaller and left inline.
|
@aponcedeleonch Thanks for the detailed review. Both pre-merge concerns are fixed in df2918e: Pi setup resolves the Anthropic path prefix before discovery and uses the same base URL for Pi requests (explicit root/custom prefixes and auto-probe); setup gives a PI_CACHE_RETENTION=long hint unless short caching was selected, with CLI help explaining that Pi cannot persist this setting. Tests cover both paths, task test/lint-fix/build pass, and the dev gateway returns 401 to the prefix probe. I also addressed the smaller inline comments or documented the model-metadata limitation there. |
Keep Anthropic discovery and requests on the same gateway path, and explain Pi's environment-only cache setting so users can opt into long retention. Tighten installation detection and document model metadata limits.
Staging advertises Anthropic thinking support in model discovery. Propagate that metadata to Pi so supported models expose thinking, while leaving fields the gateway does not provide at Pi's defaults.
Pi uses budget-based thinking by default for custom Anthropic models, but staging advertises adaptive-only models. Write the adaptive compatibility setting when discovery reports that mode so reasoning requests do not fail.
Main added Pi as an MCP and skills client after this branch added it as a gateway-only client. Reuse the existing Pi entry for both integrations to avoid a duplicate declaration and unblock CI.
aponcedeleonch
left a comment
There was a problem hiding this comment.
Thanks for working through all of the feedback. Everything I raised is addressed, and the reasoning discovery on top is a nice addition. LGTM.


Summary
thv llm setupand provide safe setup and teardown of its custom providers.Type of change
Test plan
task test)task test-e2e)task lint-fix)task build, configured Pi, and sent a minimal prompt through all 28 discovered models across both provider APIs. 27 returned a response; one returned an upstream 403 due to missing model invocation permissions. Compared every reported model size with the generated Pi configuration; all available metadata matched. One model supplies no size and therefore uses the Pi default. Also verified Pi low-thinking requests for adaptive and budget-based Anthropic models against staging.Changes
pkg/client/config.go,pkg/client/llm_gateway.go,pkg/client/llm_gateway_pi.gopkg/llm/setup.go,pkg/llmgateway/config.gopkg/client/llm_gateway_pi_test.go,pkg/llm/setup_test.gocmd/thv/app/llm.go,docs/cli/thv_llm_setup.mdDoes this introduce a user-facing change?
Yes.
thv llm setup --client piconfigures OpenAI-compatible and Anthropic models in~/.pi/agent/models.jsonusing the ToolHive token helper.Special notes for reviewers
Discovery does report per-model context sizes (
context_windowon/v1/models,max_input_tokenson/anthropic/v1/models); ToolHive writes them as PicontextWindow(e.g.gpt-5is 400,000 andgpt-5.6-solis 1,050,000). Separately, Pi defaults omittedinputto text-only and omittedmaxTokensto 16,384 output tokens. Neither discovery endpoint reports image-input or maximum-output metadata, so setup cannot override those Pi defaults from discovery; the 16,384 value is not the context size or an advertised model output limit. The OpenAI-compatible discovery response does not advertise reasoning support, so Pi also defaults those models toreasoning: false; this means Pi does not offer thinking-level controls for them. Anthropic discovery does advertise thinking modes for 11 of 12 models; those are configured withreasoning: trueand adaptive compatibility where required. These limitations are also documented inthv llm setup --help.The advertised
us.anthropic.claude-fable-5-1model currently returns 403 because its upstream role cannot invoke streaming inference; this is an external permission issue, not an integration failure. Models without reported context metadata keep the Pi default.