mirror of
https://github.com/jeffusion/gitea-ai-assistant.git
synced 2026-06-08 07:26:52 +00:00
fix(agent-kernel): address Oracle review round 2 findings
- tryParseFinalSubmission: return result when summaryMarkdown is non-empty even with empty findings - consecutiveToolFailures: append tool result to transcript before checking termination threshold - fingerprint: add buildLegacyFingerprint and dual-index existingPublished for migration compatibility - regression tests: empty findings+summary, clip newline boundary, fingerprint format migration
This commit is contained in:
@@ -209,16 +209,6 @@ export class MainAgentRunner {
|
|||||||
consecutiveToolFailures = 0;
|
consecutiveToolFailures = 0;
|
||||||
}
|
}
|
||||||
|
|
||||||
if (!result.ok && consecutiveToolFailures >= maxConsecutiveToolFailures) {
|
|
||||||
return this.finish(
|
|
||||||
sessionId,
|
|
||||||
'max_consecutive_tool_failures',
|
|
||||||
turns,
|
|
||||||
toolCalls,
|
|
||||||
messages
|
|
||||||
);
|
|
||||||
}
|
|
||||||
|
|
||||||
this.transcriptRepository.appendToolCall({
|
this.transcriptRepository.appendToolCall({
|
||||||
sessionId,
|
sessionId,
|
||||||
messageId: assistantRecord.id,
|
messageId: assistantRecord.id,
|
||||||
@@ -244,6 +234,16 @@ export class MainAgentRunner {
|
|||||||
result,
|
result,
|
||||||
},
|
},
|
||||||
});
|
});
|
||||||
|
|
||||||
|
if (!result.ok && consecutiveToolFailures >= maxConsecutiveToolFailures) {
|
||||||
|
return this.finish(
|
||||||
|
sessionId,
|
||||||
|
'max_consecutive_tool_failures',
|
||||||
|
turns,
|
||||||
|
toolCalls,
|
||||||
|
messages
|
||||||
|
);
|
||||||
|
}
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -98,4 +98,20 @@ describe('applyDeterministicPublishAdapter deduplication', () => {
|
|||||||
expect(result.findings).toHaveLength(1);
|
expect(result.findings).toHaveLength(1);
|
||||||
expect(result.findings[0].severity).toBe('high');
|
expect(result.findings[0].severity).toBe('high');
|
||||||
});
|
});
|
||||||
|
|
||||||
|
it('fingerprint migration: legacy colon-format matches new JSON tuple format', () => {
|
||||||
|
const category = 'security';
|
||||||
|
const path = 'src/auth.ts';
|
||||||
|
const line = 42;
|
||||||
|
const title = 'SQL injection';
|
||||||
|
const legacy = createHash('sha256')
|
||||||
|
.update(`${category}:${path}:${line}:${title}`)
|
||||||
|
.digest('hex')
|
||||||
|
.slice(0, 24);
|
||||||
|
const modern = createHash('sha256')
|
||||||
|
.update(JSON.stringify([category, path, line, title]))
|
||||||
|
.digest('hex')
|
||||||
|
.slice(0, 24);
|
||||||
|
expect(legacy).not.toBe(modern);
|
||||||
|
});
|
||||||
});
|
});
|
||||||
|
|||||||
@@ -34,6 +34,18 @@ function buildFingerprint(category: string, path: string, line: number, title: s
|
|||||||
.slice(0, 24);
|
.slice(0, 24);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
function buildLegacyFingerprint(
|
||||||
|
category: string,
|
||||||
|
path: string,
|
||||||
|
line: number,
|
||||||
|
title: string
|
||||||
|
): string {
|
||||||
|
return createHash('sha256')
|
||||||
|
.update(`${category}:${path}:${line}:${title}`)
|
||||||
|
.digest('hex')
|
||||||
|
.slice(0, 24);
|
||||||
|
}
|
||||||
|
|
||||||
function normalizeTitleRoot(title: string): string {
|
function normalizeTitleRoot(title: string): string {
|
||||||
return title
|
return title
|
||||||
.toLowerCase()
|
.toLowerCase()
|
||||||
@@ -129,6 +141,15 @@ export async function applyDeterministicPublishAdapter(params: {
|
|||||||
const existingPublished = new Map<string, boolean>();
|
const existingPublished = new Map<string, boolean>();
|
||||||
for (const finding of details?.findings ?? []) {
|
for (const finding of details?.findings ?? []) {
|
||||||
existingPublished.set(finding.fingerprint, finding.published);
|
existingPublished.set(finding.fingerprint, finding.published);
|
||||||
|
const legacy = buildLegacyFingerprint(
|
||||||
|
finding.category,
|
||||||
|
finding.path,
|
||||||
|
finding.line,
|
||||||
|
finding.title
|
||||||
|
);
|
||||||
|
if (legacy !== finding.fingerprint) {
|
||||||
|
existingPublished.set(legacy, finding.published);
|
||||||
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
const findings: Finding[] = normalized.map((finding) => ({
|
const findings: Finding[] = normalized.map((finding) => ({
|
||||||
|
|||||||
@@ -94,7 +94,7 @@ function tryParseFinalSubmission(text?: string): SubmittedReviewFindings | null
|
|||||||
const parsed = JSON.parse(text);
|
const parsed = JSON.parse(text);
|
||||||
if (parsed && typeof parsed === 'object') {
|
if (parsed && typeof parsed === 'object') {
|
||||||
const result = normalizeSubmission(parsed);
|
const result = normalizeSubmission(parsed);
|
||||||
if (result.findings.length > 0) return result;
|
if (result.findings.length > 0 || result.summaryMarkdown) return result;
|
||||||
}
|
}
|
||||||
} catch {}
|
} catch {}
|
||||||
return parseSubmissionFromText(text);
|
return parseSubmissionFromText(text);
|
||||||
|
|||||||
@@ -204,4 +204,13 @@ describe('review task tools', () => {
|
|||||||
expect(result).toMatchObject({ accepted: false });
|
expect(result).toMatchObject({ accepted: false });
|
||||||
expect(state.submittedReview).toEqual({ summaryMarkdown: 'Previous', findings: [] });
|
expect(state.submittedReview).toEqual({ summaryMarkdown: 'Previous', findings: [] });
|
||||||
});
|
});
|
||||||
|
|
||||||
|
test('normalizeSubmission preserves summaryMarkdown with empty findings', () => {
|
||||||
|
const result = normalizeSubmission({
|
||||||
|
summaryMarkdown: 'No issues found.',
|
||||||
|
findings: [],
|
||||||
|
});
|
||||||
|
expect(result.summaryMarkdown).toBe('No issues found.');
|
||||||
|
expect(result.findings).toEqual([]);
|
||||||
|
});
|
||||||
});
|
});
|
||||||
|
|||||||
@@ -121,4 +121,21 @@ describe('TokenCounter dynamic catalog', () => {
|
|||||||
// Should not throw
|
// Should not throw
|
||||||
counter.stopRefresh();
|
counter.stopRefresh();
|
||||||
});
|
});
|
||||||
|
|
||||||
|
test('clip respects newline boundary when possible', () => {
|
||||||
|
const counter = new TokenCounter();
|
||||||
|
const lines = ['line1', 'line2', 'line3', 'line4', 'line5'];
|
||||||
|
const text = lines.join('\n');
|
||||||
|
const budget = 3;
|
||||||
|
const clipped = counter.clip(text, budget);
|
||||||
|
expect(clipped).toContain('... [truncated');
|
||||||
|
expect(clipped.includes('\n')).toBe(true);
|
||||||
|
});
|
||||||
|
|
||||||
|
test('clip falls back to char boundary when no newline near cutoff', () => {
|
||||||
|
const counter = new TokenCounter();
|
||||||
|
const text = 'abcdefghij';
|
||||||
|
const clipped = counter.clip(text, 2);
|
||||||
|
expect(clipped).toContain('... [truncated');
|
||||||
|
});
|
||||||
});
|
});
|
||||||
|
|||||||
Reference in New Issue
Block a user