Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 4 additions & 2 deletions packages/ui/src/features/loops/components/LoopDetailView.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,6 @@ import {
import { Flex, Text } from "@radix-ui/themes";
import { useRef, useState } from "react";
import { useLoop } from "../hooks/useLoop";
import { useLoopDisplayModel } from "../hooks/useLoopDisplayModel";
import {
useDeleteLoop,
useRunLoop,
Expand All @@ -41,6 +40,7 @@ import {
nextScheduleRun,
summarizeNotificationDestinations,
} from "../loopDisplay";
import { loopEffectiveModel } from "../loopModelDefaults";
import { LoopLoadError } from "./LoopFallbacks";
import { LoopRunRow } from "./LoopRunRow";

Expand Down Expand Up @@ -265,7 +265,9 @@ function loopStatusBadgeVariant(
}

function ConfigSummarySection({ loop }: { loop: LoopSchemas.Loop }) {
const displayModel = useLoopDisplayModel(loop.runtime_adapter, loop.model);
const displayModel = loop.model
? loop.model
: `${loopEffectiveModel(loop.runtime_adapter, "")} (default)`;
const {
members,
isLoading: membersLoading,
Expand Down
4 changes: 3 additions & 1 deletion packages/ui/src/features/loops/components/LoopForm.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ import {
loopToFormValues,
normalizeLoopFormValues,
} from "../loopFormTypes";
import { loopEffectiveModel } from "../loopModelDefaults";
import { LoopBehaviorFields } from "./LoopBehaviorFields";
import { LoopContextFields } from "./LoopContextFields";
import { Field } from "./LoopFormPrimitives";
Expand Down Expand Up @@ -521,7 +522,8 @@ function ReviewList({ values }: { values: LoopFormValues }) {
<ReviewRow
label="Model"
value={`${ADAPTER_LABELS[values.runtimeAdapter]} · ${
values.model || "Default model"
values.model ||
`${loopEffectiveModel(values.runtimeAdapter, "")} (default)`
} · ${reasoning} reasoning`}
/>
<ReviewRow
Expand Down
70 changes: 51 additions & 19 deletions packages/ui/src/features/loops/components/LoopModelFields.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ import { useModelCatalog } from "@posthog/ui/features/agent-applications/hooks/u
import { SettingsOptionSelect } from "@posthog/ui/features/settings/SettingsOptionSelect";
import { Flex } from "@radix-ui/themes";
import { useMemo } from "react";
import { loopSupportedReasoningEfforts } from "../loopModelDefaults";
import { Field } from "./LoopFormPrimitives";

const ADAPTER_OPTIONS: {
Expand All @@ -16,17 +17,16 @@ const ADAPTER_OPTIONS: {
const AUTO_REASONING_VALUE = "auto";
const DEFAULT_MODEL_VALUE = "__default__";

const REASONING_EFFORT_OPTIONS: {
value: LoopSchemas.LoopReasoningEffortEnum | typeof AUTO_REASONING_VALUE;
label: string;
}[] = [
{ value: AUTO_REASONING_VALUE, label: "Auto" },
{ value: "low", label: "Low" },
{ value: "medium", label: "Medium" },
{ value: "high", label: "High" },
{ value: "xhigh", label: "Extra high" },
{ value: "max", label: "Max" },
];
const REASONING_EFFORT_LABELS: Record<
LoopSchemas.LoopReasoningEffortEnum,
string
> = {
low: "Low",
medium: "Medium",
high: "High",
xhigh: "Extra high",
max: "Max",
};

interface LoopModelFieldsProps {
adapter: LoopSchemas.LoopRuntimeAdapterEnum;
Expand All @@ -46,7 +46,9 @@ interface LoopModelFieldsProps {
* `UnifiedModelSelector`/`ReasoningLevelSelector` (which read a session's
* `SessionConfigOption`) don't apply here, so this presents the same choices
* as a dropdown against the served model catalog instead. The server validates
* the final value against the catalog in `process_task/utils.py`.
* the final value against the catalog in `process_task/utils.py`. The catalog
* is deliberately not filtered by the session composer's GLM feature flag:
* GLM is the loop fire-time default, so hiding it here would hide the default.
*/
export function LoopModelFields({
adapter,
Expand Down Expand Up @@ -86,6 +88,32 @@ export function LoopModelFields({
];
}, [catalog, model]);

// An unsupported saved effort can't reach this select: `normalizeLoopFormValues`
// clamps it to auto when the form hydrates.
const reasoningEffortOptions = useMemo(() => {
return [
Comment on lines +93 to +94

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 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)

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.

{ value: AUTO_REASONING_VALUE, label: "Auto" },
...loopSupportedReasoningEfforts(adapter, model).map((effort) => ({
value: effort,
label: REASONING_EFFORT_LABELS[effort],
})),
];
}, [adapter, model]);

const clearUnsupportedEffort = (
nextAdapter: LoopSchemas.LoopRuntimeAdapterEnum,
nextModel: string,
) => {
if (
reasoningEffort &&
!loopSupportedReasoningEfforts(nextAdapter, nextModel).includes(
reasoningEffort,
)
) {
onReasoningEffortChange(null);
}
};

return (
<Flex direction="column" gap="4">
<Field
Expand All @@ -96,9 +124,11 @@ export function LoopModelFields({
value={model || DEFAULT_MODEL_VALUE}
options={modelOptions}
placeholder="Default (recommended)"
onValueChange={(value) =>
onModelChange(value === DEFAULT_MODEL_VALUE ? "" : value)
}
onValueChange={(value) => {
const nextModel = value === DEFAULT_MODEL_VALUE ? "" : value;
onModelChange(nextModel);
clearUnsupportedEffort(adapter, nextModel);
}}
disabled={disabled}
ariaLabel="Model"
/>
Expand All @@ -109,9 +139,11 @@ export function LoopModelFields({
<SettingsOptionSelect
value={adapter}
options={ADAPTER_OPTIONS}
onValueChange={(value) =>
onAdapterChange(value as LoopSchemas.LoopRuntimeAdapterEnum)
}
onValueChange={(value) => {
const nextAdapter = value as LoopSchemas.LoopRuntimeAdapterEnum;
onAdapterChange(nextAdapter);
clearUnsupportedEffort(nextAdapter, model);
}}
disabled={disabled}
ariaLabel="Adapter"
/>
Expand All @@ -120,7 +152,7 @@ export function LoopModelFields({
<Field label="Reasoning effort" className="min-w-[180px] flex-1">
<SettingsOptionSelect
value={reasoningEffort ?? AUTO_REASONING_VALUE}
options={REASONING_EFFORT_OPTIONS}
options={reasoningEffortOptions}
onValueChange={(value) =>
onReasoningEffortChange(
value === AUTO_REASONING_VALUE
Expand Down
31 changes: 0 additions & 31 deletions packages/ui/src/features/loops/hooks/useLoopDisplayModel.ts

This file was deleted.

2 changes: 1 addition & 1 deletion packages/ui/src/features/loops/loopBuilderPrompt.ts
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@ In the config you assemble, set \`context_target\` to {"folder_id": ${JSON.strin
- Whether it may open pull requests, and how I want to hear about runs (in-app, email, or Slack).
- A short name.
4. If the loop works on a repository, resolve its GitHub integration by calling \`integrations-list\` for THIS project and use that integration's real \`github_integration_id\`. Never invent or reuse an id from memory. If this project has no GitHub integration, do NOT attach a repository or guess an id: tell me to connect GitHub for this project first, or build a report-only loop if that fits what I asked for.
5. As soon as you have a working draft and the essentials, call the PostHog MCP \`loops-review\` tool with the full assembled configuration (the same fields \`loops-create\` takes: name, instructions, runtime_adapter, triggers, behaviors, notifications, and so on). Unless a context or I say otherwise, make it a personal loop.
5. As soon as you have a working draft and the essentials, call the PostHog MCP \`loops-review\` tool with the full assembled configuration (the same fields \`loops-create\` takes: name, instructions, runtime_adapter, triggers, behaviors, notifications, and so on). Leave \`model\` and \`reasoning_effort\` unset unless I ask for a specific model; unset means PostHog picks the default model for each run. Unless a context or I say otherwise, make it a personal loop.

The \`loops-review\` card IS the review surface: it renders the whole loop for me to read and gives me a Create button. Do NOT review the loop as plain text. Never paste the drafted config into a message and ask "does this look right?", and never just narrate that it's ready and stop. The moment you have enough, call \`loops-review\`. If I ask for changes, call \`loops-review\` again with the updated config. Never call \`loops-create\` yourself: the card's Create button creates the loop once I confirm.`;
}
33 changes: 33 additions & 0 deletions packages/ui/src/features/loops/loopFormTypes.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -163,6 +163,39 @@ describe("normalizeLoopFormValues", () => {
const values = validFormValues();
expect(normalizeLoopFormValues(values)).toBe(values);
});

it.each([
{
name: "unpinned claude with an effort the default model lacks",
overrides: { model: "", reasoningEffort: "medium" },
expected: null,
},
{
name: "unpinned claude with an effort the default model supports",
overrides: { model: "", reasoningEffort: "high" },
expected: "high",
},
{
name: "pinned model supporting the effort",
overrides: { model: "claude-sonnet-5", reasoningEffort: "medium" },
expected: "medium",
},
{
name: "unpinned codex with an effort the default model lacks",
overrides: {
runtimeAdapter: "codex",
model: "",
reasoningEffort: "max",
},
expected: null,
},
] as const)(
"clamps an unsupported effort to auto: $name",
({ overrides, expected }) => {
const values = { ...validFormValues(), ...overrides };
expect(normalizeLoopFormValues(values).reasoningEffort).toBe(expected);
},
);
});

describe("formValuesToLoopWrite", () => {
Expand Down
19 changes: 16 additions & 3 deletions packages/ui/src/features/loops/loopFormTypes.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import type { LoopSchemas } from "@posthog/api-client/loops";
import { systemTimezone } from "@posthog/ui/primitives/timezone";
import { loopSupportedReasoningEfforts } from "./loopModelDefaults";

/**
* A trigger row in the create/edit form. `key` is a client-only stable
Expand Down Expand Up @@ -131,10 +132,22 @@ export function emptyLoopFormValues(): LoopFormValues {
export function normalizeLoopFormValues(
values: LoopFormValues,
): LoopFormValues {
if (values.contextTarget && values.visibility !== "team") {
return { ...values, visibility: "team" };
let normalized = values;
if (normalized.contextTarget && normalized.visibility !== "team") {
normalized = { ...normalized, visibility: "team" };
}
// A stored effort can predate a fire-time default change; carrying it into
// the form would make every save 400. It fires as auto anyway, so show that.
if (
normalized.reasoningEffort &&
!loopSupportedReasoningEfforts(
normalized.runtimeAdapter,
normalized.model,
).includes(normalized.reasoningEffort)
) {
normalized = { ...normalized, reasoningEffort: null };
}
return values;
return normalized;
}

export function loopToFormValues(loop: LoopSchemas.Loop): LoopFormValues {
Expand Down
38 changes: 38 additions & 0 deletions packages/ui/src/features/loops/loopModelDefaults.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,38 @@
import { describe, expect, it } from "vitest";
import {
loopEffectiveModel,
loopSupportedReasoningEfforts,
} from "./loopModelDefaults";

describe("loopEffectiveModel", () => {
it.each([
["claude", "", "@cf/zai-org/glm-5.2"],
["codex", "", "gpt-5"],
["claude", "claude-opus-4-8", "claude-opus-4-8"],
["codex", "gpt-5.5", "gpt-5.5"],
] as const)(
"resolves adapter %s with model %j to %s",
(adapter, model, expected) => {
expect(loopEffectiveModel(adapter, model)).toBe(expected);
},
);
});

describe("loopSupportedReasoningEfforts", () => {
it.each([
["claude", "", ["high", "max"]],
["claude", "@cf/zai-org/glm-5.2", ["high", "max"]],
["claude", "claude-sonnet-4-6", ["low", "medium", "high"]],
["claude", "claude-opus-4-8", ["low", "medium", "high", "xhigh", "max"]],
["codex", "", ["low", "medium", "high"]],
["codex", "gpt-5.5", ["low", "medium", "high", "xhigh"]],
["codex", "gpt-5.6-sol", ["low", "medium", "high", "xhigh", "max"]],
["codex", "gpt-unknown", ["low", "medium", "high"]],
["claude", "claude-unknown", ["low", "medium", "high", "xhigh", "max"]],
] as const)(
"adapter %s with model %j supports %j",
(adapter, model, expected) => {
expect(loopSupportedReasoningEfforts(adapter, model)).toEqual(expected);
},
);
});
51 changes: 51 additions & 0 deletions packages/ui/src/features/loops/loopModelDefaults.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,51 @@
import type { LoopSchemas } from "@posthog/api-client/loops";

// Mirrors DEFAULT_MODEL_BY_RUNTIME_ADAPTER and the per-model effort tables in posthog's products/tasks/backend/temporal/process_task/utils.py.
export const LOOP_DEFAULT_MODEL_BY_ADAPTER: Record<
LoopSchemas.LoopRuntimeAdapterEnum,
string
> = {
claude: "@cf/zai-org/glm-5.2",
codex: "gpt-5",
};

const ALL_EFFORTS: LoopSchemas.LoopReasoningEffortEnum[] = [
"low",
"medium",
"high",
"xhigh",
"max",
];
const LOW_TO_HIGH: LoopSchemas.LoopReasoningEffortEnum[] = [
"low",
"medium",
"high",
];

const EFFORTS_BY_MODEL: Record<string, LoopSchemas.LoopReasoningEffortEnum[]> =
{
"@cf/zai-org/glm-5.2": ["high", "max"],
"claude-opus-4-5": LOW_TO_HIGH,
"claude-sonnet-4-6": LOW_TO_HIGH,
"gpt-5": LOW_TO_HIGH,
"gpt-5.5": ["low", "medium", "high", "xhigh"],
"gpt-5.6-sol": ALL_EFFORTS,
"gpt-5.6-terra": ALL_EFFORTS,
"gpt-5.6-luna": ALL_EFFORTS,
};

export function loopEffectiveModel(
adapter: LoopSchemas.LoopRuntimeAdapterEnum,
model: string,
): string {
return model || LOOP_DEFAULT_MODEL_BY_ADAPTER[adapter];
}

export function loopSupportedReasoningEfforts(
adapter: LoopSchemas.LoopRuntimeAdapterEnum,
model: string,
): LoopSchemas.LoopReasoningEffortEnum[] {
const efforts = EFFORTS_BY_MODEL[loopEffectiveModel(adapter, model)];
if (efforts) return efforts;
return adapter === "codex" ? LOW_TO_HIGH : ALL_EFFORTS;
}
Loading