feat: reconciliation service — generated claims re-verified against source#66
feat: reconciliation service — generated claims re-verified against source#66djimit wants to merge 3 commits into
Conversation
…ource The self-improvement machinery generates claims (auto-filed GitHub issues) but never re-verifies them, so fixed problems stay open and poison downstream reasoning — by 2026-07-15, most of issues #30-#37 described code that had long been fixed. ReconciliationService closes that loop: - claim checkers keyed to the generator's title templates: LOC claims (file found by basename, stale when materially reduced or gone), test-coverage claims (matching *.test.ts lookup, conservative on fuzzy targets), execSync-timeout claims (per-call-site window scan) - reconcile() aggregates verdicts, computes generator precision (valid / verifiable), persists runs in reconciliation_runs - reconcileGitHub() lists open auto-generated issues; report-only by default, apply:true comments evidence and closes stale ones - routes: POST /api/self-improve/reconcile, GET .../reconciliation Verified live against the real repo (report-only): 8 open issues -> 4 stale (30, 31, 33, 34), 3 still valid, 1 unverifiable; measured generator precision 0.429. Notably it kept #32 open — the file is only 11% smaller than claimed — correcting this session's own earlier manual audit, which is the point of the service. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
Kilo Code Review could not run — your account is out of credits. Add credits or switch to a free model to enable reviews on this change. |
There was a problem hiding this comment.
Code Review
This pull request introduces the ReconciliationService along with corresponding API endpoints and test suites to verify auto-generated claims (such as LOC, test coverage, and execSync timeouts) against the current codebase, allowing stale GitHub issues to be closed automatically. The review feedback highlights critical performance and correctness improvements for the service, including caching file lookups and test file lists to avoid redundant disk I/O, ensuring these caches are cleared after each run to prevent stale data across API requests, and adding a fallback mechanism in the test coverage checker to correctly handle short target names.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
| private findFileByBasename(name: string): string | null { | ||
| const roots = [join(this.repoRoot, 'packages')]; | ||
| const stack = [...roots]; | ||
| while (stack.length > 0) { | ||
| const dir = stack.pop()!; | ||
| let entries; | ||
| try { entries = readdirSync(dir, { withFileTypes: true }); } catch { continue; } | ||
| for (const entry of entries) { | ||
| if (entry.isDirectory()) { | ||
| if (entry.name !== 'node_modules' && entry.name !== 'dist' && !entry.name.startsWith('.')) { | ||
| stack.push(join(dir, entry.name)); | ||
| } | ||
| } else if (entry.name === name) { | ||
| return join(dir, entry.name); | ||
| } | ||
| } | ||
| } | ||
| return null; | ||
| } |
There was a problem hiding this comment.
The findFileByBasename method performs a full recursive directory traversal of the packages directory on every single invocation. When reconciling multiple claims, this results in an
We can optimize this by caching the file map lazily on the first call, and clearing the cache at the end of each reconciliation run to prevent stale cache issues across different API requests.
private fileMap: Map<string, string> | null = null;
private testFilesCache: string[] | null = null;
private clearCache(): void {
this.fileMap = null;
this.testFilesCache = null;
}
private findFileByBasename(name: string): string | null {
if (!this.fileMap) {
this.fileMap = new Map<string, string>();
const roots = [join(this.repoRoot, 'packages')];
const stack = [...roots];
while (stack.length > 0) {
const dir = stack.pop()!;
let entries;
try { entries = readdirSync(dir, { withFileTypes: true }); } catch { continue; }
for (const entry of entries) {
if (entry.isDirectory()) {
if (entry.name !== 'node_modules' && entry.name !== 'dist' && !entry.name.startsWith('.')) {
stack.push(join(dir, entry.name));
}
} else if (entry.name.endsWith('.ts')) {
this.fileMap.set(entry.name, join(dir, entry.name));
}
}
}
}
return this.fileMap.get(name) ?? null;
}| // Significant words of the target ("execution-engine.ts" → execution-engine; "middleware layer" → middleware) | ||
| const words = target.toLowerCase().replace(/\.ts$/, '').split(/[\s/]+/) | ||
| .filter((w) => w.length > 3 && !['layer', 'implementations', 'database'].includes(w)); | ||
| const matches = testFiles.filter((f) => words.some((w) => f.toLowerCase().includes(w.replace(/s$/, '')))); |
There was a problem hiding this comment.
If the target name is short (e.g., db.ts, api.ts, cli.ts), filtering out words with length <= 3 results in an empty words array. This causes the checker to find no matching test files and incorrectly report the claim as still valid (a false positive). Adding a fallback to use the original target name when all words are filtered out resolves this issue.
// Significant words of the target ("execution-engine.ts" → execution-engine; "middleware layer" → middleware)
let words = target.toLowerCase().replace(/\\.ts$/, '').split(/[\\s/]+/)
.filter((w) => w.length > 3 && !['layer', 'implementations', 'database'].includes(w));
if (words.length === 0) {
words = [target.toLowerCase().replace(/\\.ts$/, '')];
}
const matches = testFiles.filter((f) => words.some((w) => f.toLowerCase().includes(w.replace(/s$/, ''))));| private listTestFiles(): string[] { | ||
| const dir = join(this.repoRoot, 'packages', 'server', 'src', '__tests__'); | ||
| try { return readdirSync(dir).filter((f) => f.endsWith('.test.ts')); } catch { return []; } | ||
| } |
There was a problem hiding this comment.
The listTestFiles method reads the test directory synchronously on every test coverage claim check. We can optimize this by caching the list of test files during the run.
private listTestFiles(): string[] {
if (this.testFilesCache) return this.testFilesCache;
const dir = join(this.repoRoot, 'packages', 'server', 'src', '__tests__');
try {
this.testFilesCache = readdirSync(dir).filter((f) => f.endsWith('.test.ts'));
} catch {
this.testFilesCache = [];
}
return this.testFilesCache;
}| reconcile(claims: Array<{ title: string; issueNumber?: number }>, source = 'manual'): ReconciliationReport { | ||
| const verdicts = claims.map((c) => ({ ...this.verifyClaim(c.title), issueNumber: c.issueNumber ?? null })); | ||
| return this.persistReport(verdicts, source, false, []); | ||
| } |
There was a problem hiding this comment.
To ensure that the file and test caches do not persist across different API requests (which could lead to stale data if files are modified/added/deleted by other services), clear the cache at the end of the reconcile execution using a finally block.
reconcile(claims: Array<{ title: string; issueNumber?: number }>, source = 'manual'): ReconciliationReport {
try {
const verdicts = claims.map((c) => ({ ...this.verifyClaim(c.title), issueNumber: c.issueNumber ?? null }));
return this.persistReport(verdicts, source, false, []);
} finally {
this.clearCache();
}
}| async reconcileGitHub(options: { apply?: boolean; label?: string } = {}): Promise<ReconciliationReport> { | ||
| const repo = process.env.GITHUB_REPOSITORY; | ||
| const token = process.env.GITHUB_TOKEN; | ||
| if (!repo) throw new Error('GITHUB_REPOSITORY_REQUIRED'); | ||
| if (!token) throw new Error('GITHUB_TOKEN_REQUIRED'); | ||
| const label = options.label ?? 'auto-generated'; | ||
|
|
||
| const headers = { | ||
| Authorization: `Bearer ${token}`, | ||
| Accept: 'application/vnd.github+json', | ||
| 'Content-Type': 'application/json', | ||
| }; | ||
| const listResponse = await this.fetchImpl( | ||
| `https://api.github.com/repos/${repo}/issues?state=open&labels=${encodeURIComponent(label)}&per_page=100`, | ||
| { headers }, | ||
| ); | ||
| if (!listResponse.ok) throw new Error(`GITHUB_LIST_FAILED:${listResponse.status}`); | ||
| const issues = await listResponse.json() as Array<{ number: number; title: string; pull_request?: unknown }>; | ||
|
|
||
| const verdicts: ClaimVerdict[] = issues | ||
| .filter((issue) => !issue.pull_request) | ||
| .map((issue) => ({ ...this.verifyClaim(issue.title), issueNumber: issue.number })); | ||
|
|
||
| const closed: number[] = []; | ||
| if (options.apply) { | ||
| for (const verdict of verdicts) { | ||
| if (verdict.stillValid !== false || verdict.issueNumber === null) continue; | ||
| const body = `Reconciliation (${new Date().toISOString().slice(0, 10)}): this claim no longer holds against current source.\n\n**Evidence:** ${verdict.evidence}\n\nClosed automatically by the DjimFlo reconciliation service.`; | ||
| await this.fetchImpl(`https://api.github.com/repos/${repo}/issues/${verdict.issueNumber}/comments`, { | ||
| method: 'POST', headers, body: JSON.stringify({ body }), | ||
| }); | ||
| const closeResponse = await this.fetchImpl(`https://api.github.com/repos/${repo}/issues/${verdict.issueNumber}`, { | ||
| method: 'PATCH', headers, body: JSON.stringify({ state: 'closed', state_reason: 'not_planned' }), | ||
| }); | ||
| if (closeResponse.ok) closed.push(verdict.issueNumber); | ||
| } | ||
| } | ||
|
|
||
| return this.persistReport(verdicts, `github:${repo}`, Boolean(options.apply), closed); | ||
| } |
There was a problem hiding this comment.
To ensure that the file and test caches do not persist across different API requests, clear the cache at the end of the reconcileGitHub execution using a finally block.
async reconcileGitHub(options: { apply?: boolean; label?: string } = {}): Promise<ReconciliationReport> {
const repo = process.env.GITHUB_REPOSITORY;
const token = process.env.GITHUB_TOKEN;
if (!repo) throw new Error('GITHUB_REPOSITORY_REQUIRED');
if (!token) throw new Error('GITHUB_TOKEN_REQUIRED');
const label = options.label ?? 'auto-generated';
const headers = {
Authorization: 'Bearer ' + token,
Accept: 'application/vnd.github+json',
'Content-Type': 'application/json',
};
try {
const listResponse = await this.fetchImpl(
'https://api.github.com/repos/' + repo + '/issues?state=open&labels=' + encodeURIComponent(label) + '&per_page=100',
{ headers },
);
if (!listResponse.ok) throw new Error('GITHUB_LIST_FAILED:' + listResponse.status);
const issues = await listResponse.json() as Array<{ number: number; title: string; pull_request?: unknown }>;
const verdicts: ClaimVerdict[] = issues
.filter((issue) => !issue.pull_request)
.map((issue) => ({ ...this.verifyClaim(issue.title), issueNumber: issue.number }));
const closed: number[] = [];
if (options.apply) {
for (const verdict of verdicts) {
if (verdict.stillValid !== false || verdict.issueNumber === null) continue;
const body = 'Reconciliation (' + new Date().toISOString().slice(0, 10) + '): this claim no longer holds against current source.\\n\\n**Evidence:** ' + verdict.evidence + '\\n\\nClosed automatically by the DjimFlo reconciliation service.';
await this.fetchImpl('https://api.github.com/repos/' + repo + '/issues/' + verdict.issueNumber + '/comments', {
method: 'POST', headers, body: JSON.stringify({ body }),
});
const closeResponse = await this.fetchImpl('https://api.github.com/repos/' + repo + '/issues/' + verdict.issueNumber, {
method: 'PATCH', headers, body: JSON.stringify({ state: 'closed', state_reason: 'not_planned' }),
});
if (closeResponse.ok) closed.push(verdict.issueNumber);
}
}
return this.persistReport(verdicts, 'github:' + repo, Boolean(options.apply), closed);
} finally {
this.clearCache();
}
}…edos) Claims arrive from user-controllable input (issue titles, API body), so the test-coverage and exec-timeout matchers now use indexOf/includes instead of backtracking regexes. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Same scheduler pattern as the eval nightly: default-off, armed by RECONCILIATION_NIGHTLY_ENABLED (requires GITHUB_REPOSITORY + GITHUB_TOKEN), one GitHub reconciliation per UTC day deduped against reconciliation_runs, report-only unless RECONCILIATION_NIGHTLY_APPLY=true, unref'd timer. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Summary
Pillar 2 of Closed-Loop Governance: the system stops lying to itself. The self-improvement machinery files issues but never re-verifies them — by 2026-07-15 most of #30–#37 described code that had long been fixed, and that stale data measurably poisoned analysis built on it. This service closes the generator's loop at the tracker.
Reduce complexity in <file> (<N> LOC)) — file found by basename, stale when materially reduced (>20%) or deletedAdd tests for <target>) — matching*.test.tslookup, deliberately conservative on fuzzy targets (false negatives keep issues open — the safe direction)unverifiable, left untouchedreconcile()aggregates verdicts, computes generator precision (valid ÷ verifiable claims), persists every run inreconciliation_runsreconcileGitHub()lists openauto-generatedissues (needsGITHUB_REPOSITORY+GITHUB_TOKEN); report-only by default,apply: trueposts an evidence comment and closes stale issues as not_plannedPOST /api/self-improve/reconcile(write:governance),GET /api/self-improve/reconciliationLive verification (report-only, against this repo's real issues)
The #32 verdict corrects this session's own earlier manual audit, which had called it stale — exactly the failure mode the service exists to catch.
Validation
Third leg of the loop with #61 (data supply) and #65 (enforcement).
🤖 Generated with Claude Code