Settings: Profile sampling presets (Thinking / Instruct / Coding / Custom) - #275
Closed
El-Patronum wants to merge 1 commit into
Closed
Settings: Profile sampling presets (Thinking / Instruct / Coding / Custom)#275El-Patronum wants to merge 1 commit into
El-Patronum wants to merge 1 commit into
Conversation
Adds a Profile control above Sampling in the inference popover: Thinking (temp 1.0, reasoning on), Instruct (temp 0.6, reasoning off), Coding (temp 0.6, reasoning on), Custom. Temperature and reasoning are the only knobs that differ between the named profiles, so they are the only knobs a profile writes. Top P / Top K / Presence Penalty are left alone rather than given invented per-profile values. No control is ever disabled. Every slider stays draggable while a profile is selected, and the selection is derived from the live drafts instead of being stored, so moving any slider drops the picker to Custom on its own with no state to invalidate. The three named profiles have distinct (temperature, reasoning) pairs, so the derivation is unambiguous. Also bumps the enter-choreography rowCount to 9 to match the added section; without it the last section (KV quantization) would never reach its reveal threshold.
Author
|
Closing: dropping the profile presets. The markdown-table bug fix this was split from stays open as #273. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Adds a Profile control to the inference popover, directly above Sampling, matching the existing popover layout (
InferenceSection+ segmented picker + caption hint).Split out of #273, which now carries only the unrelated markdown-table bug fix.
Profiles
Temperature and reasoning are the only knobs that differ between the named profiles, so they are the only knobs a profile writes. Top P / Top K / Presence Penalty are deliberately left untouched rather than given invented per-profile values — whatever the model and the user already have stays put.
Selecting Custom is a deliberate no-op: "Custom" describes values the user already owns, so there is nothing for it to write.
Two design points worth reviewing
1. Nothing is ever disabled. Every slider and the reasoning picker stay draggable at all times, including while a named profile is selected. A profile is a shortcut, not a lock.
2. The selection is derived, not stored. There is no
@Statefor the profile.activeProfilecomputes it from the livetemperature/reasoningModedrafts:That is what makes "the moment the user moves a slider it becomes Custom" fall out for free — there is no stored selection to invalidate, and the picker can never claim a profile whose values are no longer on screen (e.g. after
seedDraftsFromCurrentState()re-seeds from the daemon). The three named profiles have distinct(temperature, reasoning)pairs, so the derivation is unambiguous.One consequence, called out deliberately: if the user hand-dials exactly onto a profile's pair, the picker shows that profile rather than Custom. That seemed strictly better than the alternative — the state genuinely is that configuration, and it avoids "nudge a slider up and back down, still says Custom".
Temperature is compared with a tolerance well under the slider's 0.05 step rather than with
==.Incidental fix
runEnterChoreography(rowCount:)goes 8 → 9. The popover reveals sections one row at a time and each section gates onrowsVisibleCount > n; adding a section without bumping the count would leave the last one (KV quantization, now at> 8) permanently invisible.Reasoning-unsupported models
applyProfileapplies temperature, then early-returns before touching reasoning whenreasoningSupportedis false, so a profile can't fabricate a mode the model cannot honour.Proof
swift build: clean, no new warnings in the touched file.swift test: 572 passed, 1 failed —testDaemonSupervisorStartsFakeProcessAndKeepsLogsBounded, which passes in isolation (verified) and lives inMTPLXAppCore's daemon supervisor, a module this view-only diff does not touch.