From 5ae83d9bd275fc59f33dbd966cea4aeaf51e2326 Mon Sep 17 00:00:00 2001 From: Greg Pstrucha <875316+gricha@users.noreply.github.com> Date: Wed, 30 Sep 2026 10:35:31 -0700 Subject: [PATCH] fix(dedup): Allow reasoning tokens and report output limits Raise finding comparison budgets to 8,192 tokens so reasoning models can finish semantic comparisons instead of falling back to exact hashes. Report truncation as output_limit with the model and token diagnostics, and surface failed cross-location consolidation. Cover the three comparison paths and truncation reporting with regression tests based on findings from sentry-python#7780. Refs getsentry/sentry-python#7780 Co-Authored-By: GPT-6 --- packages/warden/src/output/dedup.ts | 6 +- packages/warden/src/sdk/extract.ts | 6 +- .../sentry-python-7780-dedup.json | 38 +++++++++ packages/warden/src/sdk/runtimes/pi.test.ts | 80 +++++++++++++++++++ packages/warden/src/sdk/runtimes/pi.ts | 18 ++++- packages/warden/src/sdk/runtimes/types.ts | 1 + 6 files changed, 142 insertions(+), 7 deletions(-) create mode 100644 packages/warden/src/sdk/runtimes/__fixtures__/sentry-python-7780-dedup.json diff --git a/packages/warden/src/output/dedup.ts b/packages/warden/src/output/dedup.ts index 051bc1a1..5f518a45 100644 --- a/packages/warden/src/output/dedup.ts +++ b/packages/warden/src/output/dedup.ts @@ -5,7 +5,7 @@ import type { Confidence, Finding, Severity, UsageStats } from '../types/index.j import { findingLine } from '../types/index.js'; import { escapeHtml } from '../utils/index.js'; import { getRuntime } from '../sdk/runtimes/index.js'; -import { applyMergeGroups, canUseRuntimeAuth } from '../sdk/extract.js'; +import { applyMergeGroups, canUseRuntimeAuth, FINDING_COMPARISON_MAX_TOKENS } from '../sdk/extract.js'; import type { AuxiliaryCallOptions } from '../sdk/extract.js'; import { buildJsonOutputSection, @@ -538,7 +538,7 @@ Return [] if none are duplicates.`), schema: DuplicateMatchesSchema, model: options.model, effort: options.effort, - maxTokens: 512, + maxTokens: FINDING_COMPARISON_MAX_TOKENS, maxRetries: options.maxRetries, }); @@ -788,7 +788,7 @@ Singletons (findings with no duplicates) should not appear in any group. schema: ConsolidationGroupsSchema, model: options.model, effort: options.effort, - maxTokens: 512, + maxTokens: FINDING_COMPARISON_MAX_TOKENS, maxRetries: options.maxRetries, }); diff --git a/packages/warden/src/sdk/extract.ts b/packages/warden/src/sdk/extract.ts index 571887b1..aa37ffc9 100644 --- a/packages/warden/src/sdk/extract.ts +++ b/packages/warden/src/sdk/extract.ts @@ -17,6 +17,9 @@ import { const ExtractedFindingSchema = FindingSchema.omit({ sourceSnippet: true }); +// Reasoning and the JSON answer share the output budget on Pi providers. +export const FINDING_COMPARISON_MAX_TOKENS = 8_192; + /** Pattern to match the start of findings JSON (allows whitespace after brace) */ export const FINDINGS_JSON_START = /\{\s*"findings"/; @@ -576,11 +579,12 @@ Singletons should not appear. Return [] if no findings describe the same issue.` schema: MergeGroupsSchema, model: options?.model, effort: options?.effort, - maxTokens: 512, + maxTokens: FINDING_COMPARISON_MAX_TOKENS, maxRetries: options?.maxRetries, }); if (!result.success) { + console.warn(`LLM cross-location consolidation failed, keeping all findings: ${result.error}`); return { findings, mergedCount: 0, usage: result.usage }; } diff --git a/packages/warden/src/sdk/runtimes/__fixtures__/sentry-python-7780-dedup.json b/packages/warden/src/sdk/runtimes/__fixtures__/sentry-python-7780-dedup.json new file mode 100644 index 00000000..633792c0 --- /dev/null +++ b/packages/warden/src/sdk/runtimes/__fixtures__/sentry-python-7780-dedup.json @@ -0,0 +1,38 @@ +{ + "source": "https://github.com/getsentry/sentry-python/pull/7780", + "findings": [ + { + "id": "YNL-KUW", + "severity": "high", + "confidence": "high", + "title": "A local SERVER_NAME marks public WSGI requests as localhost", + "description": "The WSGI middleware passes `SERVER_NAME` as `url_host` to `is_localhost()`. Since `SERVER_NAME` can describe the internal server and any local input makes the helper return true, a deployment with `SERVER_NAME=localhost` or `127.0.0.1` will mark requests as localhost even when `HTTP_HOST` and the client IP are public. If the inbound localhost filter uses this attribute, it can discard production traffic. Use the effective public request host instead, and add a regression test with a local `SERVER_NAME` and public `HTTP_HOST` and client IP.", + "location": { + "path": "sentry_sdk/integrations/wsgi.py", + "startLine": 24 + } + }, + { + "id": "XNA-MC6", + "severity": "high", + "confidence": "high", + "title": "Local SERVER_NAME marks public WSGI requests as localhost", + "description": "In `wsgi.py`, `is_localhost(url_host=environ[\"SERVER_NAME\"])` treats the internal bind name as the request host, so `SERVER_NAME=localhost|127.0.0.1` with a public `HTTP_HOST` still sets `sentry.is_localhost=True` and can drop production traffic via inbound filters. Pass the public host from `get_host(...)` (or omit `url_host` when `HTTP_HOST` is set), and add a regression case with local `SERVER_NAME` + public Host/IP expecting False.", + "location": { + "path": "tests/integrations/wsgi/test_wsgi.py", + "startLine": 1361 + } + }, + { + "id": "89W-DUX", + "severity": "medium", + "confidence": "high", + "title": "Client-controlled X-Forwarded-For forces localhost client_ip", + "description": "is_localhost() uses get_client_ip(), which prefers the first X-Forwarded-For hop with no use_x_forwarded_for gate, so a client can send X-Forwarded-For: 127.0.0.1 and get sentry.is_localhost=true. Prefer REMOTE_ADDR unless forwarded headers are explicitly trusted.", + "location": { + "path": "sentry_sdk/integrations/wsgi.py", + "startLine": 24 + } + } + ] +} diff --git a/packages/warden/src/sdk/runtimes/pi.test.ts b/packages/warden/src/sdk/runtimes/pi.test.ts index 47d11d23..2b08623e 100644 --- a/packages/warden/src/sdk/runtimes/pi.test.ts +++ b/packages/warden/src/sdk/runtimes/pi.test.ts @@ -8,6 +8,10 @@ import { createAgentSession, } from '@earendil-works/pi-coding-agent'; import { piRuntime } from './pi.js'; +import { mergeCrossLocationFindings } from '../extract.js'; +import { consolidateBatchFindings, deduplicateFindings, findingToExistingComment } from '../../output/dedup.js'; +import { FindingSchema } from '../../types/index.js'; +import dedupFixture from './__fixtures__/sentry-python-7780-dedup.json' with { type: 'json' }; import { createCheckoutFileTools } from './pi-file-tools.js'; import { configureWardenOffline, @@ -835,6 +839,7 @@ describe('piRuntime structured calls', () => { piMocks.listeners = []; piMocks.resourceLoaderOptions = []; piMocks.customTools = []; + piMocks.streamFunction.mockReset(); piMocks.session.agent.streamFunction = piMocks.streamFunction; piMocks.session.prompt.mockImplementation(async () => emitSuccessfulRun( assistantMessage({ content: [{ type: 'text', text: '{"ok":true}' }] }) @@ -843,6 +848,81 @@ describe('piRuntime structured calls', () => { piMocks.modelRuntime.getModels.mockReturnValue([piMocks.model]); }); + it.each(['batch', 'cross-location', 'existing-comments'] as const)( + 'deduplicates PR findings with reasoning longer than 512 tokens during %s comparison', + async (kind) => { + const findings = z.tuple([FindingSchema, FindingSchema, FindingSchema]).parse(dedupFixture.findings); + const [original, duplicate, independent] = findings; + const output = kind === 'existing-comments' + ? '[{"findingIndex":1,"existingIndex":1}]' + : '[[1,2]]'; + // Reproduce a provider that spends the old 512-token budget before it can emit JSON. + piMocks.session.prompt.mockImplementation(async () => { + await piMocks.session.agent.streamFunction(piMocks.model, { messages: [] }, {}); + }); + piMocks.streamFunction.mockImplementation((_model, _context, options: { maxTokens?: number }) => { + const truncated = (options.maxTokens ?? 0) < 2_048; + emitSuccessfulRun(assistantMessage({ + stopReason: truncated ? 'length' : 'stop', + content: [{ type: 'text', text: truncated ? '' : output }], + })); + }); + const options = { runtime: 'pi' as const, model: 'openai/gpt-test', effort: 'high' as const }; + + if (kind === 'existing-comments') { + const result = await deduplicateFindings( + [duplicate, independent], [findingToExistingComment(original)!], options, + ); + expect(result.newFindings.map((finding) => finding.id)).toEqual([independent.id]); + expect(result.duplicateActions).toMatchObject([{ originalFindingId: duplicate.id, matchType: 'semantic' }]); + } else { + const result = kind === 'batch' + ? await consolidateBatchFindings(findings, options) + : await mergeCrossLocationFindings(findings, options); + expect(result.findings.map((finding) => finding.id)).toEqual([original.id, independent.id]); + expect(result.findings[0]?.additionalLocations).toContainEqual(duplicate.location); + } + }, + ); + + it.each(['auxiliary', 'synthesis'] as const)('reports output truncation and its budget for %s calls', async (kind) => { + piMocks.session.prompt.mockImplementation(async () => emitSuccessfulRun(assistantMessage({ + stopReason: 'length', + content: [{ type: 'text', text: '[{"finding' }], + usage: { ...assistantMessage().usage, output: 512 }, + }))); + let spans: TraceSpan[] | undefined; + await Sentry.startSpan({ op: 'test', name: 'truncated comparison' }, async (span) => { + const recorder = startTraceRecorder(span); + const request = { + prompt: 'Compare findings', schema: z.array(z.unknown()), model: 'openai/gpt-test', maxTokens: 512, + }; + const result = await withTraceRecorder(recorder, () => kind === 'auxiliary' + ? piRuntime.runAuxiliary({ ...request, task: 'deduplication' }) + : piRuntime.runSynthesis({ ...request, task: 'consolidation' })); + expect(result).toMatchObject({ + success: false, + error: expect.stringContaining('output token limit'), + usage: { outputTokens: 512 }, + }); + if (!result.success) { + expect(result.error).toContain('finish_reason=length'); + expect(result.error).toContain('max_tokens=512'); + expect(result.error).toContain('output_tokens=512'); + expect(result.error).toContain('gpt-test-2026'); + } + spans = recorder?.snapshot(); + }); + expect(spans).toEqual(expect.arrayContaining([expect.objectContaining({ + op: 'gen_ai.invoke_agent', + attributes: expect.objectContaining({ + 'error.type': 'output_limit', + 'gen_ai.request.max_tokens': 512, + 'gen_ai.response.finish_reasons': ['length'], + }), + })])); + }); + it.each(['auxiliary', 'synthesis'] as const)('honors the output-token limit on %s provider requests', async (kind) => { piMocks.session.prompt.mockImplementation(async () => { await piMocks.session.agent.streamFunction(piMocks.model, { messages: [] }, { diff --git a/packages/warden/src/sdk/runtimes/pi.ts b/packages/warden/src/sdk/runtimes/pi.ts index 8a176711..4c97f708 100644 --- a/packages/warden/src/sdk/runtimes/pi.ts +++ b/packages/warden/src/sdk/runtimes/pi.ts @@ -471,6 +471,7 @@ function statusFromPiMessage(message: AssistantMessage, hitMaxTurns: boolean): S case 'stop': return 'success'; case 'length': + return 'output_limit'; case 'toolUse': return 'provider_error'; case 'error': @@ -484,15 +485,25 @@ function statusFromPiMessage(message: AssistantMessage, hitMaxTurns: boolean): S } } -function normalizePiResult(run: PiPromptResult): SkillRunResult | undefined { +function normalizePiResult(run: PiPromptResult, maxTokens?: number): SkillRunResult | undefined { const message = run.lastAssistant; if (!message) { return undefined; } const errors = message.errorMessage ? [message.errorMessage] : []; + const status = statusFromPiMessage(message, run.hitMaxTurns); + if (status === 'output_limit') { + const details = [ + 'finish_reason=length', + `model=${message.responseModel ?? message.model}`, + ...(maxTokens !== undefined ? [`max_tokens=${maxTokens}`] : []), + `output_tokens=${message.usage.output}`, + ]; + errors.unshift(`Pi response reached the output token limit (${details.join(', ')}). Increase the budget to allow reasoning and a complete answer.`); + } return { - status: statusFromPiMessage(message, run.hitMaxTurns), + status, text: textFromAssistant(message), errors, usage: run.usage, @@ -918,6 +929,7 @@ async function runStructured( ...(request.agentName ? { 'gen_ai.agent.name': request.agentName } : {}), ...(request.task ? { 'warden.ai.task': request.task } : {}), ...(request.model ? { 'gen_ai.request.model': request.model } : {}), + ...(request.maxTokens !== undefined ? { 'gen_ai.request.max_tokens': request.maxTokens } : {}), 'gen_ai.output.type': 'json', }, }, @@ -943,7 +955,7 @@ async function runStructured( timeout: request.timeout, parentSpan: span, }); - const result = normalizePiResult(run); + const result = normalizePiResult(run, request.maxTokens); if (!result) { span.setAttribute('error.type', 'missing_response'); return { success: false, error: 'Pi runtime returned no response', usage: run.usage }; diff --git a/packages/warden/src/sdk/runtimes/types.ts b/packages/warden/src/sdk/runtimes/types.ts index 94e9bf64..b265dd34 100644 --- a/packages/warden/src/sdk/runtimes/types.ts +++ b/packages/warden/src/sdk/runtimes/types.ts @@ -26,6 +26,7 @@ export type SkillRunStatus = | 'provider_error' | 'auth_error' | 'turn_limit' + | 'output_limit' | 'budget_limit' | 'aborted' | 'structured_output_error';