[Fryuni]

Problem

OhMyPi (omp) model presets set the model for every role (default, task, plan, smol, …) plus a thinking level, but neither the Pi RPC bridge nor omp's ACP mode exposes them. Inside T3 Code, an omp thread could only change its main model. omp's own /modelpreset switch writes the user's global config, and every running omp reloads it, so using it would switch every thread and the user's terminal at once.

Change

All within packages/provider-pi; no contract, client, or orchestration changes.

• Discovery. When the Pi binary reports omp/<version>, discovery reads omp config get modelPresets --json and lists each preset as a model preset:<name> with a "Preset" qualifier. Its Thinking select comes from the preset's main model, with "Auto" for omp's auto. Config reads pass the instance's --profile and time out on their own, so a slow read loses only the presets, not the discovered models. Presets omp would reject, or that have no default role, are left out.
• Per-thread isolation. Every Pi session launches with its own overlay file in T3's cache, listed last in PI_CONFIG_FILES. omp reads it like --config, and plain Pi ignores it. It stays empty until a turn selects a preset. Selecting a preset re-reads it from omp and rewrites the overlay (atomic rename). The overlay holds the preset's roles, null tombstones for roles it leaves out (as /modelpreset switch clears them), and its thinking level. The turn then sets the main model and thinking over RPC. Selecting a plain model empties the overlay. omp's global config is never written.
• Switching works mid-thread through the normal composer path. A restart-based design was dropped because message.dispatch never consults planSelectionTransition. Overlay state is tracked separately from the applied model, so a failed switch is repaired by the next selection.
• Text generation has no per-model filter, so a preset chosen there uses only its main model.
• ADR 0025 records the reasoning, accepted gaps, and upstream-sync guidance. docs/user/providers-pi.md gains an "OhMyPi Model Presets" section.

Out of scope, filed as #79: omp slash-command autocomplete and command_output rendering in the Pi provider.

Scope and approval

Requested by the maintainer (@Fryuni). The design (Pi RPC rather than ACP or a native provider, per-thread overlay, presets as picker entries) was agreed in a grilling session before implementation.

Verification

• Focused tests in presets.test.ts, status.test.ts, textGeneration.test.ts, and adapter.test.ts cover:

  • discovery, and dropping presets omp would reject;
  • overlay contents, including tombstones;
  • preset → preset → plain-model switching;
  • recovery after a failed switch;
  • auto thinking, thinking abbreviations, and the configured-level fallback after a resume;
  • a missing preset failing visibly, and plain Pi rejecting presets;
  • --profile forwarding and the stalled-read timeout.

  All 141 provider-pi tests pass, and vp run check:changed is green.

• Live checks against the maintainer's omp 18.5.1, with ~/.omp/agent/config.yml checksum unchanged afterwards:

  • an RPC process launched with an overlay starts on the overlay's main model instead of the global default;
  • a running RPC process picks up an atomically renamed overlay within 1.5s, both via --config and via PI_CONFIG_FILES.

• All 8 Pi replay fixtures in PiReplayFixtures.integration.test.ts pass locally.

• Reviewed in five rounds by GPT-6.1 Sol reviewers; the final round was confident on every slice.

• Not checked: a real T3 client rendering the picker (no UI code changed; existing subProvider rendering is reused), Windows, and omp aliases (@slow) as a preset's main model, which fail with a clear error by design.

• Known limit: omp reloads a rewritten overlay about 200ms later. The main model and thinking switch immediately over RPC, and role models switch from the next role lookup.

Implemented by Claude Opus 5.5 in Claude Code (T3 Code harness), reviewed by GPT-6.1 Sol via Codex.

🤖 Generated with Claude Code

[chatgpt-codex-connector]

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review	Status	Commit	Review trigger
📝 Code Review	✅ Completed 2026-10-27T00:28:34.441821Z	c07c801	New commits

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

[fryuni-proval]

Thanks for adding thread-local OhMyPi presets and covering the recovery paths alongside normal model switching.

Overview

Summary

This adds preset discovery to the Pi model catalog, applies role configuration through per-session overlays rather than global config, and lets background generation use a preset's main model. I recommend requesting changes for three boundary cases affecting session startup, context budgeting, and discovery fallback.

Good points

🟢 The failed-switch regression verifies that a rejected set_model does not leave preset roles active on the next plain-model turn (packages/provider-pi/src/server/adapter.test.ts:1986).
🟢 Background-generation coverage exercises preset resolution through generateCommitMessage, checking both the structured result and the concrete RPC model selection (packages/provider-pi/src/server/textGeneration.test.ts).

Main Issues

⚫️ Bound overlay filenames — packages/provider-pi/src/server/adapter.ts:481 uses the full session ID as a filename, so valid long MCP-derived IDs can prevent any omp session from opening.

⚫️ Resolve preset capacity before budgeting — packages/provider-pi/src/server/adapter.ts:2475–2479 returns no context window until the preset has already been applied, but history budgeting happens before that first turn.

⚫️ Separate discovery deadlines — packages/provider-pi/src/server/status.ts:175–190 awaits optional preset discovery inside the RPC timeout, allowing a stalled config read to discard an otherwise successful inventory refresh.

Additional Notes

Validation was limited to static inspection; tests were not run.

[fryuni-proval]

Thanks for tightening the session-overlay launch path and addressing the filename-length issue from the earlier review.

Overview

Summary

This follow-up bounds overlay filenames and moves overlay configuration from --config to PI_CONFIG_FILES, removing the startup binary-detection probe. I found no new actionable issues in this push; this follow-up does not supersede the remaining requests in the earlier review.

Good points

🟢 packages/provider-pi/src/server/adapter.ts:478 caps the session-ID portion of the filename and appends a full-ID hash, keeping both the final and temporary filenames below the filesystem component-length limit.
🟢 packages/provider-pi/src/server/mcpInjection.test.ts:25 checks that an inherited PI_CONFIG_FILES value is retained before the session overlay, protecting the launch builder's environment-composition contract.

Main Issues

No new findings to add for this push.

Additional Notes

Validation was limited to static inspection; tests were not run.

[fryuni-proval]

Thanks, @Fryuni, for following up on overlay startup and first-turn context budgeting.

Overview

Summary

This push bounds overlay filenames, passes overlays through PI_CONFIG_FILES, and resolves preset-to-model mappings during thread registration so capacity is available before the first turn. No additional changes requested for this push; this assessment is limited to the follow-up changes.

Good points

🟢 Preset mappings are populated during registration and consulted by capacity lookups before a preset runs (packages/provider-pi/src/server/adapter.ts:2219–2241, packages/provider-pi/src/server/adapter.ts:2492–2498), addressing the first-turn budgeting gap.
🟢 The long-session-ID regression exercises actual overlay creation and checks both the bounded basename and initialized contents (packages/provider-pi/src/server/adapter.test.ts:1983–1997), protecting startup for deeply composed session identifiers.

Main Issues

No new issues identified in this push. Previously reported issues outside the push scope are not repeated here.

Additional Notes

Validation was limited to static inspection; tests were not run.

[fryuni-proval]

Thanks for following through on the earlier feedback and adding regressions for the boundary cases.

Overview

Summary

This push addresses the three earlier concerns: bounded overlay filenames (packages/provider-pi/src/server/adapter.ts:478), preset context resolution during thread registration (packages/provider-pi/src/server/adapter.ts:2219), and optional preset enrichment outside the RPC deadline (packages/provider-pi/src/server/status.ts:438). No new blocking issues found; I recommend approval based on static review.

Good points

🟢 The pre-turn context-window regression checks a different preset from the initially selected one, covering lookup before either preset is applied (packages/provider-pi/src/server/adapter.test.ts:2004).
🟢 The discovery regression spends 25 test-clock seconds on RPC before stalling preset discovery, directly exercising the overlapping-deadline failure while checking that authentication and discovered models survive (packages/provider-pi/src/server/status.test.ts:231).

Main Issues

None found in this follow-up push.

Additional Notes

Validation was limited to static inspection; tests were not run.

[fryuni-proval]

Thanks for following through on the remaining context-budgeting and discovery-timeout fixes, and for adding targeted regression coverage.

Overview

Summary

This follow-up resolves preset-to-model mappings during thread registration, before the first turn, and moves optional preset discovery outside the RPC discovery deadline. These changes address the two remaining requests from the earlier review. I found no new blockers and recommend merging based on static inspection.

Good points

🟢 packages/provider-pi/src/server/adapter.test.ts:2001 checks a preset's context window before any turn starts, covering the ordering needed for initial history budgeting.
🟢 packages/provider-pi/src/server/status.test.ts:250 deliberately consumes 25 seconds of the RPC budget before stalling preset discovery, then verifies that the separate timeout preserves authentication and discovered models.

Main Issues

No new actionable findings in this push.

Additional Notes

Validation was limited to static inspection; tests were not run.