Skip to content

perf(forms): stop duplicating form schema in collector context - #36

Open
dmourot wants to merge 2 commits into
mainfrom
dmourot/athens-v4
Open

dmourot wants to merge 2 commits into
mainfrom
dmourot/athens-v4

Conversation

@dmourot

@dmourot dmourot commented Jun 27, 2026

Copy link
Copy Markdown
Contributor

The form collector context was embedding the full form schema as JSON, duplicating field names, types, descriptions and allowed values that already reach the agent through the update_<name> tool params. The context now sends only the requiredness information the tool omits: the base required list (the tool sends [] for incremental filling) and the per-field requiredWhen conditions (stripped from the tool schema to stay JSON-Schema-legal). For large forms the embedded schema was the single biggest chunk of the request, so this roughly halves the prompt in those cases. Tests cover the new requiredness rules and guard against the full schema being re-embedded.

🤖 Generated with Claude Code

dmourot and others added 2 commits June 26, 2026 15:18
The field schema (names, types, descriptions, allowed values) already
reaches the agent via the update_<name> tool params. The context now
sends only the requiredness rules the tool omits: the base `required`
list and the per-field `requiredWhen` conditions. For large forms the
embedded schema was the single biggest chunk of the request, so this
roughly halves the prompt for those cases.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The four JSDoc comments on FormFieldCondition, FormFieldSchema.requiredWhen,
FormCollectorSchema, and toWireParameters still claimed the full schema reaches
the agent via the dynamic context. After the previous commit the context sends
only the requiredness rules (required list + requiredWhen conditions), so the
comments are corrected to say that.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@dmourot

dmourot commented Jun 27, 2026 •

Copy link
Copy Markdown
Contributor Author

Code review

Found 1 issue (fixed in 3202951 per the fix >= 50 request):

  1. Four JSDoc comments still claimed the full form schema reaches the agent via the dynamic context, but this PR changed the context to send only the requiredness rules (required list + requiredWhen conditions). The comments on FormFieldCondition, FormFieldSchema.requiredWhen, FormCollectorSchema, and toWireParameters were left stale and now describe behavior the PR removed (bug due to the JSON.stringify(schema) → JSON.stringify(requirementRules) change in contextProvider).

* Stripped from the LLM tool's `parameters` (see {@link toWireParameters}), but the
* full schema, `requiredWhen` included, still reaches the agent via the dynamic
* context so it knows when each field becomes required.

Two other candidates were checked and dismissed as pre-existing (not introduced by this PR): a field listed in both required and requiredWhen reads as always-required, and the "Some fields are conditionally required" sentence is emitted unconditionally. Both behaviors existed before this change.

🤖 Generated with Claude Code

- If this code review was useful, please react with 👍. Otherwise, react with 👎.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant