improvement(agent): allow variable references in reasoning effort, verbosity, and thinking level

[waleedlatif1]

Summary

• Reasoning Effort, Verbosity, and Thinking Level become editable comboboxes instead of select-only dropdowns, so they accept <block.output> / <variable.x> / {{ENV_VAR}} references — same as the Model field directly above them. Each keeps its per-model option list.
• All three stay visible when model itself holds a reference, since the concrete model id is only known at execution time and can't be matched against the static capability list.
• Resolved levels are trimmed and lower-cased at sanitizeRequest, and a reference that resolves to nothing reads as unset rather than sending an empty string the provider rejects (and stays distinct from an explicit none).

One deliberate behavior change

Levels are no longer discarded for a model absent from models.ts.

The Model field is already an editable combobox on staging, so a model newer than our catalogue is typeable today and routes by pattern (gpt-5.2 → openai, claude-opus-6 → anthropic) and executes normally — but supportsReasoningEffort('gpt-5.2') is false, so its tuning level was silently dropped. Every model release re-created that papercut until someone updated the catalogue.

Catalogued models that genuinely lack the field (claude-opus-4-6 + reasoningEffort) and every dynamic-provider id (ollama/…, openrouter/…) keep the protective drop, so the "stale level after switching models" path is unchanged.

Everything else is behavior-neutral.

Testing

Full apps/sim suite green (18,397). Coverage added for: reference resolution into all three fields (block output, workflow variable, env var); level normalization including the none-vs-unset distinction; catalogued/uncatalogued/dynamic-provider drop behavior; combobox conversion and reference-model visibility; and level redaction.

Verified rather than assumed:

• old vs new condition evaluation across all 204 catalogued model ids plus edge cases, for all three capability lists — 0 divergences; only the reference case flips to visible
• containsReference — 0 false positives across all 204 model ids and all level values
• serializer: these are mode: 'advanced', so with advanced collapsed the condition isn't consulted at all — that path is byte-identical
• audited all 12 repo sites branching on the dropdown subblock type
• traced the empty-string case through all six thinkingLevel consumers
• each new test verified to fail with its change reverted

Not done

Not exercised in a browser — no one has typed a reference into these fields or run a workflow with a reference-bound level.

Checklist

• Code follows project style guidelines
• Self-reviewed my changes
• Tests added/updated and passing
• No new warnings introduced
• I confirm that I have read and agree to the terms outlined in the Contributor License Agreement (CLA)

[vercel]

The latest updates on your projects. Learn more about Vercel for GitHub.

1 Skipped Deployment

Project	Deployment	Actions	Updated (UTC)
docs	Skipped		Aug 4, 2026 1:42am

[cursor]

PR Summary

Medium Risk
Touches provider request sanitization and agent tuning behavior; catalogued vs uncatalogued model handling is a deliberate behavior change for newer models, with broad test coverage but limited manual UI verification noted in the PR.

Overview
Agent Reasoning Effort, Verbosity, and Thinking Level move from dropdowns to editable comboboxes, so workflows can bind them with <block.output>, <variable.x>, or {{ENV_VAR}} like the Model field while still offering per-model option lists.

Visibility uses getModelCapabilityCondition and containsReference: when model is a reference, tuning fields stay visible instead of being hidden by static capability lists. Literal models still gate on capability as before.

Execution path: VariableResolver resolves references into all three fields; sanitizeRequest trims/lowercases levels and treats empty resolved values as unset (distinct from explicit none). Capability stripping now applies only to catalogued models that are known not to support a field—levels for models not in models.ts are forwarded so providers can accept or reject them.

Safety: describeModelLevel / isKnownModelLevelValue redact unrecognized level strings in logs (e.g. Anthropic ignore warnings) so resolved secrets are not echoed.

^{Reviewed by Cursor Bugbot for commit 9aa8fe0. Configure here.}

[greptile-apps]

Greptile Summary

This PR makes agent tuning fields editable and resolves their reference-bound values at execution time. It also normalizes resolved values and changes capability filtering so unknown static models retain tuning settings, while dynamic-provider models continue using protective capability drops.

• Converts reasoning effort, verbosity, and thinking level fields to editable comboboxes.
• Keeps tuning fields visible when the selected model is reference-bound.
• Normalizes resolved tuning values before provider dispatch.
• Adds coverage for reference resolution, visibility, normalization, capability filtering, and redaction behavior.

Confidence Score: 4/5

The PR should not merge until the outstanding silent removal of provider-supported tuning levels from dynamic-provider model requests is addressed.

Dynamic-provider IDs are classified as known while their discovered entries expose no tuning capabilities, causing requested reasoning, verbosity, or thinking settings to be cleared before provider dispatch even when the underlying provider model supports them.

Files Needing Attention: apps/sim/providers/index.ts, apps/sim/providers/models.ts, apps/sim/providers/utils.ts

Important Files Changed

Filename	Overview
apps/sim/providers/index.ts	Normalizes tuning fields and preserves them for unknown static models, but the previously reported silent drop for capable dynamic-provider models remains.
apps/sim/providers/models.ts	Adds known-model classification that treats dynamic-provider prefixes as catalogued even though dynamically discovered entries carry empty capability metadata.
apps/sim/providers/utils.ts	Adds model-reference visibility support and safe descriptions for model-level diagnostic values.
apps/sim/blocks/blocks/agent.ts	Converts three agent tuning fields to editable comboboxes and keeps them visible for dynamically bound model IDs.
apps/sim/lib/workflows/sanitization/references.ts	Adds detection for block, workflow-variable, and environment-variable references used by dynamic field visibility.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart LR
  A[Agent tuning field] --> B[Resolve block, variable, or environment reference]
  B --> C[Trim and lowercase value]
  C --> D{Model classification}
  D -->|Unknown static model| E[Forward tuning value]
  D -->|Catalogued or dynamic-provider model| F{Capability declared}
  F -->|Yes| E
  F -->|No| G[Clear tuning value]
  E --> H[Provider request]
  G --> H

Loading

_{Reviews (7): Last reviewed commit: "refactor(providers): drop the sanitizer'..." | Re-trigger Greptile}

[waleedlatif1]

@greptile

[waleedlatif1]

@cursor review

[cursor]

✅ Bugbot reviewed your changes and found no new issues!

Comment @cursor review or bugbot run to trigger another review on this PR

^{Reviewed by Cursor Bugbot for commit 034aec7. Configure here.}

[waleedlatif1]

Addressing the two points in the last summary.

1. "Sanitization clears levels when the resolved model is absent from Sim's capability lists, without warning" — valid, fixed in 2a7a055.

This was a real gap and my own reasoning was inconsistent: I argued against silently dropping undeclared values, while the pre-existing supportsX drop did exactly that — and this PR made it far more reachable by newly exposing these fields for reference-bound models. A run whose reference resolved to a model outside models.ts lost its requested level in silence and fell back to the model default.

Dropping stays the behavior, because it is genuinely protective — a provider with no such parameter rejects the whole request, and we can't distinguish "model we haven't catalogued yet" from "model that truly has no such field". But it is no longer silent: sanitizeRequest now logs the field, model, and discarded value. Covered by tests, including that it stays quiet when no level was requested.

2. "Undeclared values for capable models continue to be forwarded" — still deliberate.

Reasoning unchanged from the resolved thread on providers/index.ts: our per-model lists populate the pickers and can lag a provider that has started accepting a new level, so rejecting on them would refuse values the API would have taken. And silently substituting the default is the worse failure for the eval sweeps this PR exists for — it produces confidently wrong numbers instead of an error. Forwarding surfaces the provider's own 400, which names the accepted values, and there is now a warning logged alongside it.

Net: every path that discards or forwards a non-obvious level is observable. Full suite green at 18,393.

[waleedlatif1]

@greptile

[waleedlatif1]

@cursor review

[waleedlatif1]

Both points addressed in fe52248.

Security — resolved level content in logs. Valid, and self-inflicted. The diagnostics I added last round logged the raw value. Since these fields now take {{ENV_VAR}} and block references, a mistyped reference resolves the secret into the level, and the warning would have written it to server logs in plaintext. Good catch.

Fixed by gating what gets echoed: a level is logged only when the catalogue declares it somewhere (isKnownModelLevelValue, built once from every model's reasoningEffort / verbosity / thinking values) or it is an auto / none sentinel. Anything else logs as [redacted N chars] — still enough to tell a stray level from a resolved blob. Test asserts a secret-shaped value never appears anywhere in the log calls.

Dynamic models losing levels. You were right, and I had the distinction wrong. I was treating "absent from the capability list" as "known to be incapable", but those are different: a model the catalogue has never seen is unknown, and a reference is exactly how a newly released model arrives before we catalogue it.

So the drop is now conditioned on isKnownModelId:

• catalogued model that lacks the field (claude-opus-4-6 + reasoningEffort) → still dropped, still protective, now logged
• dynamic-provider id (ollama/…, openrouter/…) → still dropped, unchanged
• model absent from the catalogue entirely (gpt-6-unreleased) → forwarded, provider decides

That makes the behavior consistent with the forwarding argument from the earlier thread rather than contradicting it. All three cases have tests.

Suite green at 18,397; typecheck and boundary gates clean.

[waleedlatif1]

@greptile

[waleedlatif1]

@cursor review

[waleedlatif1]

On the remaining point — dynamic-provider ids still taking the drop path.

Confirmed the underlying fact: zero ollama/, vllm/, litellm/, openrouter/, fireworks/, together/, or baseten/ ids appear in any capability list (39 catalogued reasoning models, none of them dynamic-provider). So levels are always dropped for those providers. Notably the LiteLLM adapter already has payload.reasoning_effort = request.reasoningEffort written and waiting — currently unreachable for litellm/* models because the sanitizer strips it first.

Leaving it as-is in this PR, deliberately:

1. Pre-existing on staging. This PR neither introduced nor widened it — dynamic-provider ids behave exactly as they did before. The in-scope part of the complaint (that the discard was invisible) is fixed: it now logs.
2. Changing it carries a concrete regression risk this finding doesn't weigh. Forwarding means sending reasoning_effort to arbitrary OpenAI-compatible servers. Some ignore unknown params, some 400. The scenario that breaks is real and common: pick gpt-5, set a level, switch the model to ollama/llama3 — the stored level persists and is serialized, and today it is safely dropped.
3. It is a product decision, not a defect. "Should a user be able to set a reasoning level on a model whose capabilities we cannot validate?" deserves its own PR and real verification against a live Ollama/OpenRouter endpoint, not a drive-by change inside a var-reference PR.

Happy to do it as a follow-up if we want dynamic-provider models to accept tuning levels — it is roughly a one-line predicate change plus provider-by-provider testing.

Cursor Bugbot: no bugs on this commit. Open review threads: 0.

[waleedlatif1]

@greptile

[waleedlatif1]

@cursor review

[cursor]

✅ Bugbot reviewed your changes and found no new issues!

1 issue from previous review remains unresolved.

Comment @cursor review or bugbot run to trigger another review on this PR

^{Reviewed by Cursor Bugbot for commit fe52248. Configure here.}

[waleedlatif1]

Rejecting the round-5 security finding. I implemented it, then checked whether it actually holds — it doesn't.

A model id is not secret, and this codebase already treats it as non-secret everywhere. In this same function, 12 lines below the warning in question:

logger.info('API key resolved', {
  provider: providerId,
  model: request.model,        // verbatim, every single request
  workspaceId: request.workspaceId,
  isBYOK,
})

and in the adjacent catch:

logger.error('Failed to resolve API key:', { provider: providerId, model: request.model, ... })

Plus model: request.model logged verbatim in the baseten and together adapters, and model returned in the agent block's own output, which is persisted to execution logs and rendered in the UI.

So redacting the model in two warning lines, while the very next log statement prints it verbatim on every request, protects nothing. It is theater that costs real diagnostic value — for a self-hosted Ollama or vLLM setup, which model is the single most useful field when debugging why a level was dropped, and this change would have replaced it with ollama/[redacted 33 chars].

The premise is also weak on its own terms: the model id is an identifier that must by definition be transmitted to a third-party API. It is not a credential.

Reverted, along with the now-unused isStaticModelId helper it needed.

The level redaction from fe52248 stays, because the reasoning there is different and does hold: level values are not logged anywhere else, and redacting an unrecognized one costs nothing diagnostically (real levels still print in full — only unparseable junk is reduced to a length, and "it wasn't a valid level" is all you learn from it either way).

Current state: 4 commits, all CI green, 0 open threads, Bugbot clean, 18,397 tests passing.

[waleedlatif1]

@greptile

[waleedlatif1]

@cursor review

[cursor]

✅ Bugbot reviewed your changes and found no new issues!

Comment @cursor review or bugbot run to trigger another review on this PR

^{Reviewed by Cursor Bugbot for commit a4c7abb. Configure here.}

[waleedlatif1]

Round 6: 0 open threads, Cursor Bugbot passing with no findings, all CI green. Two points remain in the summary; declining both, with reasons.

1. "Redaction allowlists common level strings globally, leaving reference-resolved secrets with those exact values visible."

The allowlist is not open-ended. Enumerated from the catalogue, it is 9 fixed lowercase words, longest 8 characters:

disabled, enabled, high, low, max, medium, minimal, none, xhigh

For this to leak, a secret would have to be exactly one of those nine words. At that point echoing it reveals nothing — an attacker already has the full vocabulary, and there is no entropy to protect. Redaction exists to keep high-entropy resolved content out of logs, which it does: anything outside those nine tokens is reduced to a length. Widening it further would only degrade diagnostics, since these are precisely the values an operator needs to read.

2. Dynamic-provider levels still discarded. Third time raised; declining for the reasons already on the record — pre-existing on staging and unchanged by this PR, forwarding risks 400s on OpenAI-compatible servers for a workflow carrying a stale level after a model switch, and it is a product decision about dynamic providers that deserves its own PR with live Ollama/OpenRouter verification. The in-scope half — that the discard was invisible — is fixed and logged.

Stopping the review loop here. Six rounds produced three real defects, all fixed:

• levels silently cleared for uncatalogued models → now logged, and uncatalogued models forward instead (2a7a055, fe52248)
• sanitizer diagnostics echoing resolved secret content → redacted (fe52248)
• Anthropic's unsupported-thinking warning echoing the same content one layer downstream → redacted via a shared describeModelLevel gate (a4c7abb)

and two findings I could not substantiate (model ids as secrets; this allowlist point), both answered with evidence rather than code changes.

Remaining before merge: this has not been exercised in a browser — no one has typed a reference into these fields or run a workflow with a reference-bound level.

[waleedlatif1]

Trimmed scope in 9aa8fe0 after an audit against the original ask.

Removed the two sanitizer diagnostics (dropUnsupportedLevel, warnOnUnrecognizedLevel) and their tests — 211 lines out, 11 in. They were added to answer review rounds rather than to serve the feature, they logged server-side where the workflow author never sees them, and they carried the redaction that leaked resolved content before it was caught. The surprising case they narrated — a level discarded for a model newer than the catalogue — is fixed at the source instead.

Kept, after checking each on merit rather than by scope:

• normalizeModelLevel — the feature creates a new input class (a reference resolving to '' or 'High'); without it '' becomes a provider 400
• describeModelLevel + Anthropic redaction — a pre-existing log this feature newly exposes to resolved reference content
• the uncatalogued-model forwarding — a genuine fix, now documented in the description as the PR's one deliberate behavior change

Total diff is down from 645 to 441 insertions. Suite green at 18,397; typecheck clean.

[waleedlatif1]

@greptile

[waleedlatif1]

@cursor review

[cursor]

✅ Bugbot reviewed your changes and found no new issues!

Comment @cursor review or bugbot run to trigger another review on this PR

^{Reviewed by Cursor Bugbot for commit 9aa8fe0. Configure here.}

[waleedlatif1]

Final status after the scope cut.

All 11 checks passing · 0 open threads · Cursor Bugbot clean · mergeable.

Greptile holds at 4/5 on a single point: dynamic-provider ids (ollama/…, openrouter/…, litellm/…) keep the protective capability drop. This is its fourth consecutive round raising it with no new information, so I am stopping the loop rather than continuing to re-trigger.

Declining it for the reasons already on the record: it is pre-existing on staging and unchanged by this PR; forwarding would send reasoning_effort to arbitrary OpenAI-compatible servers and risks 400s for a workflow carrying a stale level after a model switch; and "should a user set tuning levels on models whose capabilities we cannot validate?" is a product decision deserving its own PR with live Ollama/OpenRouter verification.

Worth noting the review did not re-raise the diagnostics removed in 9aa8fe0, which confirms they were not load-bearing.

Seven rounds produced three real defects, all fixed:

• levels silently cleared for uncatalogued models → fixed at the source
• sanitizer diagnostics echoing resolved secret content → the diagnostics are gone entirely
• Anthropic unsupported-thinking warning echoing the same content → redacted via describeModelLevel

and two findings that did not survive verification (model ids as secrets; the 9-word redaction allowlist), both answered with evidence.

Outstanding before merge: not exercised in a browser. No one has typed a reference into these fields or run a workflow with a reference-bound level.