omit falsy models in agent form by dimaMachina · Pull Request #2906 · inkeep/agents

dimaMachina · 2026-03-30T20:05:22Z

changeset-bot · 2026-03-30T20:05:28Z

🦋 Changeset detected

Latest commit: 3c38720

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 10 packages

Name	Type
@inkeep/agents-manage-ui	Patch
@inkeep/agents-api	Patch
@inkeep/agents-cli	Patch
@inkeep/agents-core	Patch
@inkeep/agents-email	Patch
@inkeep/agents-mcp	Patch
@inkeep/agents-sdk	Patch
@inkeep/agents-work-apps	Patch
@inkeep/ai-sdk-provider	Patch
@inkeep/create-agents	Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

vercel · 2026-03-30T20:05:29Z

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

Project	Deployment	Actions	Updated (UTC)
agents-api	Ready	Preview, Comment	Mar 30, 2026 8:28pm
agents-manage-ui	Ready	Preview, Comment	Mar 30, 2026 8:28pm

1 Skipped Deployment

Project	Deployment	Actions	Updated (UTC)
agents-docs	Skipped		Mar 30, 2026 8:28pm

pullfrog · 2026-03-30T20:07:16Z

TL;DR — Prevents the agent form from submitting model configuration objects when the user hasn't selected a model. Previously, empty model fields (base, structuredOutput, summarizer) were always included in the payload; now they are omitted if their model value is falsy.

Key changes

Omit empty model configs in MyModelsSchema — Adds a .transform() step to the Zod schema that conditionally includes base, structuredOutput, and summarizer only when their model field is truthy, preventing blank model objects from reaching the API.
Fix test assertions for optional model configs — Updates validation.test.ts to use optional chaining (?.) when accessing providerOptions on model configs that may now be undefined.
Add changeset for @inkeep/agents-manage-ui — Patch-level changeset documenting the fix.

_{Summary ｜ 3 files ｜ 4 commits ｜ base: main ← dimaMachina-patch-4}

Before: The agent form always sent all three model config objects (base, structuredOutput, summarizer) in the payload — even when no model was selected — resulting in empty/falsy entries persisted to the backend.
After: A .transform() on MyModelsSchema checks each model config's .model field and only includes it if truthy, keeping the submitted payload clean.

The fix chains a .transform() onto the existing z.strictObject(). Each of the three model slots is destructured and conditionally spread back using short-circuit evaluation (base.model && { base }). Any remaining properties are passed through via rest spread. The test file was updated to use optional chaining since model config properties may now be absent.

validation.ts · validation.test.ts · omit-falsy-models.md

^{｜ View workflow run ｜ Triggered by Pullfrog ｜ Using Claude Opus ｜ 𝕏}

dimaMachina · 2026-03-30T20:08:07Z

@claude add a changeset

inkeep-internal-ci · 2026-03-30T20:08:29Z

Claude finished @dimaMachina's task in 47s —— View job

Adding changeset

Analyze PR changes
Create changeset file
Commit and push

Added a patch changeset for @inkeep/agents-manage-ui: "Omit falsy model configs in agent form to prevent empty model objects in payload"
| Branch: dimaMachina-patch-4

Co-authored-by: Dimitri POSTOLOV <[email protected]>

pullfrog

Clean, targeted fix. The transform correctly omits model sections with falsy model values, preventing empty model configurations from being submitted. Downstream consumers (API validation, DAL, processModels in serialize.ts) all handle absent sections safely via optional chaining and their own empty-checks. One minor nit below.

^{｜ View workflow run ｜ Using Claude Opus ｜ 𝕏}

pullfrog · 2026-03-30T20:13:00Z

+  .transform(({ base, structuredOutput, summarizer, ...value }) => {
+    return {
+      ...value,
+      ...(base.model && { base }),
+      ...(structuredOutput.model && { structuredOutput }),
+      ...(summarizer.model && { summarizer }),
+    };


Nit: strictObject only defines base, structuredOutput, and summarizer — destructuring all three means value is always {}. The spread is a no-op and adds a small cognitive overhead. Consider simplifying:

Suggested change

.transform(({ base, structuredOutput, summarizer, ...value }) => {

return {

...value,

...(base.model && { base }),

...(structuredOutput.model && { structuredOutput }),

...(summarizer.model && { summarizer }),

};

.transform(({ base, structuredOutput, summarizer }) => ({

...(base.model && { base }),

...(structuredOutput.model && { structuredOutput }),

...(summarizer.model && { summarizer }),

}));

pullfrog · 2026-03-30T20:13:00Z

+  .transform(({ base, structuredOutput, summarizer, ...value }) => {
+    return {
+      ...value,
+      ...(base.model && { base }),


Worth noting: if a user sets providerOptions on a section but leaves model empty, the entire section (including providerOptions) is silently dropped here. This is arguably correct — providerOptions without a model is meaningless — but it differs from processModels in serialize.ts which preserves the full models object when any section has non-empty providerOptions. In practice this shouldn't matter since processModels runs after the Zod transform and won't see dropped sections anyway, but wanted to flag the asymmetry.

claude

PR Review Summary

(2) Total Issues | Risk: Low

🟡 Minor (1) 🟡

Inline Comments:

🟡 Minor: validation.test.ts:113-115 Test doesn't validate the new omission behavior — existing tests have populated models, missing explicit test for empty model omission

💭 Consider (1) 💭

Inline Comments:

💭 Consider: validation.ts:81-88 Redundant ...value spread — strictObject only defines 3 fields, so value is always {}

💡 APPROVE WITH SUGGESTIONS

Summary: Clean, targeted fix that correctly omits model sections with falsy model values, preventing empty model configurations from being submitted to the API. The transform logic is correct — empty strings are falsy and whitespace is already trimmed by the upstream ModelSettingsSchema. The downstream processModels function in serialize.ts handles undefined sections safely via optional chaining.

The suggestions above are non-blocking improvements: (1) simplifying the no-op spread for readability, and (2) adding explicit test coverage for the new omission behavior to catch regressions. Both were also flagged by pullfrog's prior review. Nice work! 🎉

Discarded (2)

Location	Issue	Reason Discarded
`validation.ts:65`	Type/runtime mismatch — transform makes properties optional at runtime but TypeScript type remains required	The API schema (`ModelSchema` in agents-core) already has all three properties as optional, so runtime optionality matches API expectations. The test file's optional chaining is defensive programming, not evidence of a type problem. The downstream `processModels` in serialize.ts casts to `AgentModels
`validation.ts:81-87`	Whitespace-only model strings are truthy and won't be omitted	`ModelSettingsSchema` applies `.trim()` during parsing (line 222 in agents-core schemas.ts), so whitespace-only strings become empty strings before the transform runs. Empty strings are correctly omitted.

Reviewers (3)

Reviewer	Returned	Inline Comments	Discarded
`pr-review-tests`	3	1	2
`pr-review-types`	2	0	2
`pr-review-standards`	0	0	0
Total	5	1	4

Note: pr-review-standards found no issues after deep analysis — confirmed the transform logic is correct for empty strings and whitespace.

claude · 2026-03-30T20:30:26Z

+  .transform(({ base, structuredOutput, summarizer, ...value }) => {
+    return {
+      ...value,
+      ...(base.model && { base }),
+      ...(structuredOutput.model && { structuredOutput }),
+      ...(summarizer.model && { summarizer }),
+    };
+  });


💭 Consider: Redundant ...value spread

Issue: strictObject only defines base, structuredOutput, and summarizer — destructuring all three means value is always {}. The spread is a no-op and adds cognitive overhead.

Why: Minor readability improvement — readers may wonder what other properties are being preserved.

Fix: Simplify (non-blocking):

.transform(({ base, structuredOutput, summarizer }) => ({ ...(base.model && { base }), ...(structuredOutput.model && { structuredOutput }), ...(summarizer.model && { summarizer }), }));

Refs:

Zod strictObject docs — strictObject disallows extra keys, so ...value is guaranteed empty

claude · 2026-03-30T20:30:40Z

+      expect(result.data.models.base?.providerOptions).toBe(undefined);
+      expect(result.data.models.structuredOutput?.providerOptions).toBe(undefined);
+      expect(result.data.models.summarizer?.providerOptions).toBe(undefined);


🟡 Minor: Test doesn't validate the new omission behavior

Issue: This test has all three model sections with non-empty model strings (lines 91-100), so the transform keeps all sections rather than omitting them. The optional chaining (?.) masks this gap — assertions pass regardless of whether sections are present.

Why: If the transform logic breaks (e.g., omits models when it shouldn't, or keeps them when it shouldn't), no test would catch it.

Fix: Add explicit tests for the new behavior:

it('should omit model sections when model field is empty', () => { const result = FullAgentFormSchema.safeParse({ id: '_', name: '_', statusUpdates: {}, contextConfig: { id: '_', contextVariables: '', headersSchema: '' }, models: { base: { model: '' }, structuredOutput: { model: '' }, summarizer: { model: '' }, }, subAgents: {}, externalAgents: {}, teamAgents: {}, tools: {}, }); expect(result.success).toBe(true); expect(result.data?.models.base).toBeUndefined(); expect(result.data?.models.structuredOutput).toBeUndefined(); expect(result.data?.models.summarizer).toBeUndefined(); });

Refs:

validation.ts:81-88 — transform being tested

itoqa · 2026-03-30T21:41:00Z

Ito Test Report ✅

12 test cases ran. 12 passed.

The unified run passed all 12 of 12 test cases with zero failures, confirming stable save behavior and payload serialization across agent and sub-agent model settings in a non-production local environment. Key findings were that cleared/inherited model fields were correctly omitted and stayed omitted after reload/navigation (including back-forward, refresh, mobile, rapid-save, and two-tab stale-update flows), explicit model selections persisted when set, and provider-options handling was robust with malformed JSON and literal null blocked by validation while empty values were normalized without null serialization or save regressions.

✅ Passed (12)

Category	Summary	Screenshot
Adversarial	Malformed JSON is rejected by validation and blocks save; prior blocker was environment instability, not product logic.
Adversarial	Literal `null` in provider options is rejected with `Cannot be null`; saving succeeds after correction.
Adversarial	Rapid save actions converged to one successful final state; the editor stayed stable and dirty state cleared.
Adversarial	Tab A cleared advanced state and saved, then stale Tab B saved a different model update; after reloading both tabs the final persisted state was consistent and deterministic (GPT-5.4 Nano), with successful save responses and no malformed-state regression observed.
Edge	Saving with all root models empty keeps inheritance-only state and reload stability.
Edge	Empty provider options are normalized without null serialization or save failure.
Edge	Back/forward + refresh flow preserves intended model state without stale resurrection.
Edge	On 390x844 viewport, agent settings remained usable; clearing structured output and saving succeeded with no layout break.
Logic	After reload, cleared advanced model keys remain omitted on subsequent save.
Logic	After clearing all sub-agent model fields and saving, captured request body omitted sub-agent models entirely, and reload confirmed inherited state with no stale values.
Happy-path	Base model saves while cleared advanced model keys are omitted from payload.
Happy-path	Sub-agent Base/Structured/Summarizer were set to explicit non-inherited models, save succeeded, and values persisted after reload.

Commit: 3c38720

View Full Run

Tell us how we did: Give Ito Feedback

@dimaMachina

* omit falsy models in agent form * Apply suggestion from @dimaMachina * Add changeset for agents-manage-ui patch Co-authored-by: Dimitri POSTOLOV <[email protected]> * fix typecheck --------- Co-authored-by: claude[bot] <41898282+claude[bot]@users.noreply.github.com> Co-authored-by: Dimitri POSTOLOV <[email protected]>

omit falsy models in agent form

a2767bf

dimaMachina commented Mar 30, 2026

View reviewed changes

Comment thread agents-manage-ui/src/components/agent/form/validation.ts Outdated

Apply suggestion from @dimaMachina

ce24c20

vercel Bot temporarily deployed to Preview – agents-docs March 30, 2026 20:05 Inactive

dimaMachina marked this pull request as ready for review March 30, 2026 20:05

vercel Bot deployed to Preview – agents-manage-ui March 30, 2026 20:07 View deployment

vercel Bot deployed to Preview – agents-api March 30, 2026 20:08 View deployment

Add changeset for agents-manage-ui patch

19bedce

Co-authored-by: Dimitri POSTOLOV <[email protected]>

github-actions Bot deleted a comment from claude Bot Mar 30, 2026

vercel Bot deployed to Preview – agents-manage-ui March 30, 2026 20:10 View deployment

vercel Bot deployed to Preview – agents-docs March 30, 2026 20:11 View deployment

pullfrog Bot reviewed Mar 30, 2026

View reviewed changes

vercel Bot deployed to Preview – agents-api March 30, 2026 20:14 View deployment

fix typecheck

3c38720

vercel Bot temporarily deployed to Preview – agents-docs March 30, 2026 20:26 Inactive

vercel Bot deployed to Preview – agents-manage-ui March 30, 2026 20:28 View deployment

vercel Bot deployed to Preview – agents-api March 30, 2026 20:28 View deployment

claude Bot reviewed Mar 30, 2026

View reviewed changes

github-actions Bot deleted a comment from claude Bot Mar 30, 2026

dimaMachina added this pull request to the merge queue Mar 30, 2026

Merged via the queue into main with commit e222844 Mar 30, 2026
23 of 24 checks passed

dimaMachina deleted the dimaMachina-patch-4 branch March 30, 2026 20:50

claude Bot mentioned this pull request Mar 30, 2026

Support nested files and folders for Skills #2719

Merged

Conversation

dimaMachina commented Mar 30, 2026

Uh oh!

changeset-bot Bot commented Mar 30, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

🦋 Changeset detected

Uh oh!

vercel Bot commented Mar 30, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

pullfrog Bot commented Mar 30, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Key changes

Uh oh!

dimaMachina commented Mar 30, 2026

Uh oh!

inkeep-internal-ci Bot commented Mar 30, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Adding changeset

Uh oh!

pullfrog Bot left a comment

Choose a reason for hiding this comment

Uh oh!

pullfrog Bot Mar 30, 2026

Choose a reason for hiding this comment

Uh oh!

pullfrog Bot Mar 30, 2026

Choose a reason for hiding this comment

Uh oh!

claude Bot left a comment

Choose a reason for hiding this comment

PR Review Summary

🟡 Minor (1) 🟡

💭 Consider (1) 💭

💡 APPROVE WITH SUGGESTIONS

Uh oh!

claude Bot Mar 30, 2026

Choose a reason for hiding this comment

Uh oh!

claude Bot Mar 30, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

itoqa Bot commented Mar 30, 2026

Ito Test Report ✅

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

changeset-bot Bot commented Mar 30, 2026 •

edited

Loading

vercel Bot commented Mar 30, 2026 •

edited

Loading

pullfrog Bot commented Mar 30, 2026 •

edited

Loading

inkeep-internal-ci Bot commented Mar 30, 2026 •

edited

Loading