Skip to content

fix(core): avoid LM Studio grammar failure #10275

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
mredbishop wants to merge 2 commits into
base: main
Choose a base branch
from
+15 −1
Open

fix(core): avoid LM Studio grammar failure #10275

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
4bdf969
fix(core): avoid LM Studio grammar failure
mredbishop Aug 27, 2026
3c43b25
Merge branch 'main' into fix/lmstudio-grammar-schema
mredbishop Aug 27, 2026
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
15 changes: 15 additions & 0 deletions packages/core/src/tools/report-findings.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,21 @@ function displayOf(result: { returnDisplay: unknown }): FindingsResultDisplay {
}

describe('ReportFindingsTool', () => {
it('does not constrain finding summaries in its tool schema', () => {
const tool = new ReportFindingsTool();
const schema = tool.schema.parametersJsonSchema as {
properties: {
findings: {
items: { properties: { summary: Record<string, unknown> } };
};
};
};

expect(
schema.properties.findings.items.properties.summary,
).not.toHaveProperty('maxLength');
});

it('reports findings as a findings_list display with counts in llmContent', async () => {
const result = await run({
level: 'high',
Expand Down
1 change: 0 additions & 1 deletion packages/core/src/tools/report-findings.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -162,7 +162,6 @@ const FINDING_ITEM_SCHEMA = {
},
summary: {
type: 'string',
maxLength: 2000,
description: 'One sentence stating the defect.',
Comment on lines 163 to 165

@qwen-code-ci-bot qwen-code-ci-bot Aug 27, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

[Suggestion] This removes the 2,000-character cap from the schema that BaseDeclarativeTool.validateToolParams validates against (Ajv enforces maxLength), so client-side enforcement of summary length is dropped for every provider, not just the LM Studio endpoint this targets. Verified by probe against the real tool and validator:

summary len=2500: PR schema -> ACCEPTED | base schema -> REJECTED

The codebase already has a layer built for exactly this class of OpenAI-compatible gateway breakage: relaxSchemaForFunctionCalling (packages/core/src/utils/schemaConverter.ts), applied to tool schemas in convertGeminiToolsToOpenAI, whose design note (PR 7315) says the constraint is relaxed on the wire only while client-side validateToolParams still enforces the full source schema. Also note the five sibling constraints in this same item schema (id 64, file 4096, failureScenario 4000, category 64, outcomeNote 1000, plus maxItems: 50) remain, so the change neither preserves the cap where it works nor closes the nested-constraint class that caused this failure.

Consider keeping maxLength: 2000 here and stripping it only at the wire layer for OpenAI-compatible endpoints (extending relaxSchemaForFunctionCalling per the PR 7315 pattern), or moving the cap into validateToolParamValues so client-side enforcement survives — and updating the new test to pin the wire shape accordingly. If the wire-relaxation route is taken, please add the converter/schemaConverter test asserting the relaxed report_findings wire schema carries no maxLength on summary while the source schema keeps it, and confirm it goes red when the relaxation step is removed.

— qwen3.8-max via Qwen Code /review (v0.22.2)

@qwen-code-ci-bot qwen-code-ci-bot Aug 27, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

[Suggestion] R1-1: Still standing — this code is unchanged since the previous round. Removing maxLength: 2000 from the source schema drops client-side (Ajv) enforcement of summary length for every provider, not just the LM Studio endpoint this targets: BaseDeclarativeTool.validateToolParams validates exactly this schema, and the wire-only relaxation layer relaxSchemaForFunctionCalling (packages/core/src/utils/schemaConverter.ts) — the PR 7315 pattern of relaxing constraints on the wire while keeping client-side enforcement — was not used.

The five sibling constraints in this same item schema (id 64, file 4096, failureScenario 4000, category 64, outcomeNote 1000) plus maxItems: 50 remain, so the change neither preserves the cap where it works nor closes the nested-constraint class that caused the LM Studio grammar failure. Round 1 verified by probe against the real tool and validator: summary len=2500: PR schema -> ACCEPTED | base schema -> REJECTED.

Consider keeping maxLength: 2000 here and stripping it only at the wire layer for OpenAI-compatible endpoints (extending relaxSchemaForFunctionCalling per the PR 7315 pattern), or moving the cap into validateToolParamValues so client-side enforcement survives — and updating the new test to pin the wire shape accordingly. If the wire-relaxation route is taken, add the converter/schemaConverter test asserting the relaxed report_findings wire schema carries no maxLength on summary while the source schema keeps it, and confirm it goes red when the relaxation step is removed.

— qwen3.8-max via Qwen Code /review (v0.22.2)

},
shortSummary: {
Expand Down
Loading