feat(loops): Align model fields with GLM fire-time default#3740
feat(loops): Align model fields with GLM fire-time default#3740charlesvien wants to merge 2 commits into
Conversation
|
Merging to
After your PR is submitted to the merge queue, this comment will be automatically updated with its status. If the PR fails, failure details will also be posted here |
|
React Doctor found no issues in the changed files. 🎉 Reviewed by React Doctor for commit |
Prompt To Fix All With AIFix the following 2 code review issues. Work through them one at a time, proposing concise fixes.
---
### Issue 1 of 2
packages/ui/src/features/loops/loopModelDefaults.ts:44-45
**Unknown models allow every effort**
When a selected catalog model has an identifier absent from `EFFORTS_BY_MODEL`, this fallback enables all five reasoning levels. The selected model and effort are then submitted without compatibility normalization, so choosing an unsupported level such as `xhigh` or `max` causes the API to reject the loop create or update.
### Issue 2 of 2
packages/ui/src/features/loops/components/LoopModelFields.tsx:93-99
**Legacy invalid effort remains submitted**
When an unpinned loop retains an effort unsupported by its current fire-time default, this code preserves that effort and cleanup runs only after a model or adapter change. Saving an unrelated edit therefore resubmits the invalid model-effort combination and the API rejects the update.
Reviews (1): Last reviewed commit: "align loop form with glm default model" | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| ): LoopSchemas.LoopReasoningEffortEnum[] { | ||
| return EFFORTS_BY_MODEL[loopEffectiveModel(adapter, model)] ?? ALL_EFFORTS; |
There was a problem hiding this comment.
When a selected catalog model has an identifier absent from EFFORTS_BY_MODEL, this fallback enables all five reasoning levels. The selected model and effort are then submitted without compatibility normalization, so choosing an unsupported level such as xhigh or max causes the API to reject the loop create or update.
Rule Used: When implementing new features, ensure that the UI... (source)
Learned From
PostHog/posthog#32595
PostHog/posthog#32677
Prompt To Fix With AI
This is a comment left during a code review.
Path: packages/ui/src/features/loops/loopModelDefaults.ts
Line: 44-45
Comment:
**Unknown models allow every effort**
When a selected catalog model has an identifier absent from `EFFORTS_BY_MODEL`, this fallback enables all five reasoning levels. The selected model and effort are then submitted without compatibility normalization, so choosing an unsupported level such as `xhigh` or `max` causes the API to reject the loop create or update.
**Rule Used:** When implementing new features, ensure that the UI... ([source](https://app.greptile.com/posthog-org-19734/-/custom-context?memory=5d57f0af-0be1-44de-8885-055f27e2885f))
**Learned From**
[PostHog/posthog#32595](https://github.com/PostHog/posthog/pull/32595)
[PostHog/posthog#32677](https://github.com/PostHog/posthog/pull/32677)
How can I resolve this? If you propose a fix, please make it concise.| const reasoningEffortOptions = useMemo(() => { | ||
| const supported = loopSupportedReasoningEfforts(adapter, model); | ||
| const efforts = | ||
| reasoningEffort && !supported.includes(reasoningEffort) | ||
| ? [...supported, reasoningEffort] | ||
| : supported; | ||
| return [ |
There was a problem hiding this comment.
When an unpinned loop retains an effort unsupported by its current fire-time default, this code preserves that effort and cleanup runs only after a model or adapter change. Saving an unrelated edit therefore resubmits the invalid model-effort combination and the API rejects the update.
Rule Used: When implementing new features, ensure that the UI... (source)
Learned From
PostHog/posthog#32595
PostHog/posthog#32677
Prompt To Fix With AI
This is a comment left during a code review.
Path: packages/ui/src/features/loops/components/LoopModelFields.tsx
Line: 93-99
Comment:
**Legacy invalid effort remains submitted**
When an unpinned loop retains an effort unsupported by its current fire-time default, this code preserves that effort and cleanup runs only after a model or adapter change. Saving an unrelated edit therefore resubmits the invalid model-effort combination and the API rejects the update.
**Rule Used:** When implementing new features, ensure that the UI... ([source](https://app.greptile.com/posthog-org-19734/-/custom-context?memory=5d57f0af-0be1-44de-8885-055f27e2885f))
**Learned From**
[PostHog/posthog#32595](https://github.com/PostHog/posthog/pull/32595)
[PostHog/posthog#32677](https://github.com/PostHog/posthog/pull/32677)
How can I resolve this? If you propose a fix, please make it concise.1 participant
Problem
PostHog/posthog#73068 makes GLM 5.2 the fire-time default for loops that don't pin a model. The loop UI didn't know about fire-time defaults at all: the detail view showed the session default from
ReportModelResolver(wrong even before, it showed opus while the backend fired sonnet-5), and the effort dropdown offered all five levels even when the effective model only supports a subset, which the API rejects with a 400.Changes
loopModelDefaults.tsmirrors the backend's per-adapter default models and per-model supported efforts. The detail view and review step now show the real fire-time default ("@cf/zai-org/glm-5.2 (default)"), the effort dropdown only offers what the effective model supports (Auto/High/Max for GLM) and picking a model that doesn't support the current effort resets it to Auto. A saved effort from an older default stays selectable so existing loops don't render a broken select. The loop builder prompt now tells the agent to leave model and effort unset unless asked.useLoopDisplayModelis deleted.How did you test this?
loopModelDefaults.test.tscovers default resolution and effort narrowing per adapter/model.@posthog/uitypecheck and biome. No manual app run.Automatic notifications