voyage/lib/review/findings-schema.mjs
Kjell Tore Guttormsen b149538d43 feat(voyage): S9 — NW1 reviewer-output schema contract (TDD, ungated)
Codify the /trekreview Phase 5 reviewer JSON contract as a validated schema so
main validates each reviewer's output instead of merely JSON.parse-ing it, and
re-asks on schema failure (not just parse failure). Retires the fragility at
trekreview.md:202-204. No Workflow dependency (S8 tier 1, ships regardless).

New lib/review/findings-schema.mjs (3-layer Content -> Raw-text -> CLI shim,
reusing result.mjs error-shape + rule-catalogue RULE_KEYS/SEVERITY_VALUES):
- validateFindings(payload): hard errors on load-bearing fields the dedup
  triplet + verdict depend on — file, rule_key (in catalogue), severity (enum),
  line (integer >= 0); accumulates all errors; per-finding location.
- extractFindingsBlock(text): last fenced ```json block (the :202 contract).
- validateReviewerOutput(text): extract + parse + schema, unifying parse and
  schema failures under one bounded re-ask path.
Stable codes: FINDINGS_NOT_OBJECT/_NOT_ARRAY/_NO_JSON_BLOCK/_PARSE_ERROR (top),
FINDING_MISSING_FILE/_MISSING_RULE_KEY/_UNKNOWN_RULE_KEY/_BAD_SEVERITY/_BAD_LINE
(per-finding). Descriptive fields + unknown keys tolerated (forward-compat).

Phase 5 prose: replace "parse last json block; on parse error re-emit" with
"validate against findings-schema; on failure re-ask conforming JSON, bounded
N=2; never feed unvalidated findings to the coordinator".

TDD: 27 failing tests first, then minimal code to pass. Suite 606 -> 647
(645 pass / 2 skip / 0 fail). claude plugin validate clean (only the known
root-CLAUDE.md warning). Plan: docs/W1-narrow-wins-plan.md S9.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01LqBYc8Ltrk7LipyJmGxXiB
2026-06-18 14:06:07 +02:00

175 lines
6.7 KiB
JavaScript
Raw Blame History

This file contains ambiguous Unicode characters

This file contains Unicode characters that might be confused with other characters. If you think that this is intentional, you can safely ignore this warning. Use the Escape button to reveal them.

// lib/review/findings-schema.mjs
// Reviewer-output JSON schema contract for /trekreview Phase 5 (NW1).
//
// brief-conformance-reviewer and code-correctness-reviewer each emit a trailing
// fenced `json` block of shape:
//
// { "reviewer": "<name>", "findings": [ { id, severity, rule_key, file, line,
// brief_ref, title, detail,
// recommended_action }, ... ] }
//
// This module codifies that contract so main can VALIDATE each reviewer's JSON
// (not merely JSON.parse it) and re-ask on *schema* failure as well as parse
// failure, replacing the fragile "parse the last json block" contract that used
// to live in commands/trekreview.md (the :202204 prose).
//
// Load-bearing fields (the downstream dedup triplet + verdict severity) are hard
// errors: file, rule_key, severity, line. Unknown rule_keys are errors too — the
// catalogue is the contract. Descriptive fields (title/detail/recommended_action/
// brief_ref) and unknown top-level keys are tolerated (forward-compat, mirroring
// review-validator.mjs).
//
// 3-layer pattern (Content → Raw-text → CLI shim) mirroring the other validators.
import { readFileSync, existsSync } from 'node:fs';
import { issue, fail } from '../util/result.mjs';
import { RULE_KEYS, SEVERITY_VALUES } from './rule-catalogue.mjs';
// The fields main + the coordinator depend on. Descriptive fields are not here
// on purpose: a missing recommended_action should not trigger a re-ask.
export const FINDING_REQUIRED_FIELDS = Object.freeze([
'severity',
'rule_key',
'file',
'line',
]);
// Last fenced ```json … ``` block in a reviewer's output. The contract pins the
// JSON block as the LAST fence so prose above it never confuses the parser.
const JSON_FENCE_GLOBAL = /```json[ \t]*\r?\n([\s\S]*?)```/gi;
/**
* Extract the inner body of the LAST fenced `json` block in `text`.
* @param {string} text
* @returns {string|null} the JSON source, or null if no json fence is present.
*/
export function extractFindingsBlock(text) {
if (typeof text !== 'string') return null;
JSON_FENCE_GLOBAL.lastIndex = 0;
let last = null;
let m;
while ((m = JSON_FENCE_GLOBAL.exec(text)) !== null) {
last = m[1];
}
return last;
}
function validateFinding(finding, index, errors) {
const loc = `findings[${index}]`;
if (finding === null || typeof finding !== 'object' || Array.isArray(finding)) {
errors.push(issue('FINDING_NOT_OBJECT', `${loc} is not an object`, undefined, loc));
return;
}
if (typeof finding.file !== 'string' || finding.file.length === 0) {
errors.push(issue('FINDING_MISSING_FILE', `${loc}.file must be a non-empty string`, undefined, loc));
}
if (typeof finding.rule_key !== 'string' || finding.rule_key.length === 0) {
errors.push(issue('FINDING_MISSING_RULE_KEY', `${loc}.rule_key must be a non-empty string`, undefined, loc));
} else if (!RULE_KEYS.has(finding.rule_key)) {
errors.push(issue(
'FINDING_UNKNOWN_RULE_KEY',
`${loc}.rule_key "${finding.rule_key}" is not in the rule catalogue`,
'Use a rule_key from lib/review/rule-catalogue.mjs',
loc,
));
}
if (typeof finding.severity !== 'string' || !SEVERITY_VALUES.includes(finding.severity)) {
errors.push(issue(
'FINDING_BAD_SEVERITY',
`${loc}.severity must be one of ${SEVERITY_VALUES.join('|')}, got ${JSON.stringify(finding.severity)}`,
undefined,
loc,
));
}
if (typeof finding.line !== 'number' || !Number.isInteger(finding.line) || finding.line < 0) {
errors.push(issue(
'FINDING_BAD_LINE',
`${loc}.line must be an integer ≥ 0, got ${JSON.stringify(finding.line)}`,
'Use 0 for file-scoped findings without a specific line.',
loc,
));
}
}
/**
* Validate an already-parsed reviewer-output payload against the schema.
* Accumulates every error (so a re-ask can name all problems at once).
* @param {unknown} payload
* @returns {import('../util/result.mjs').Result}
*/
export function validateFindings(payload) {
if (payload === null || typeof payload !== 'object' || Array.isArray(payload)) {
return fail(issue('FINDINGS_NOT_OBJECT', `Reviewer output must be a JSON object, got ${Array.isArray(payload) ? 'array' : typeof payload}`));
}
const errors = [];
const warnings = [];
if (typeof payload.reviewer !== 'string' || payload.reviewer.length === 0) {
warnings.push(issue('FINDINGS_MISSING_REVIEWER', 'Reviewer output should carry a non-empty "reviewer" name'));
}
if (!Array.isArray(payload.findings)) {
errors.push(issue('FINDINGS_NOT_ARRAY', `Field "findings" must be an array, got ${typeof payload.findings}`));
return { valid: false, errors, warnings, parsed: payload };
}
for (let i = 0; i < payload.findings.length; i++) {
validateFinding(payload.findings[i], i, errors);
}
return { valid: errors.length === 0, errors, warnings, parsed: payload };
}
/**
* Validate a reviewer's raw output text: extract the last json fence, parse it,
* then schema-validate. Parse-stage failures get stable codes so they flow
* through the same bounded re-ask path as schema failures.
* @param {string} rawText
* @returns {import('../util/result.mjs').Result}
*/
export function validateReviewerOutput(rawText) {
const block = extractFindingsBlock(rawText);
if (block === null) {
return fail(issue(
'FINDINGS_NO_JSON_BLOCK',
'No trailing fenced ```json block found in reviewer output',
'Reviewers must end their output with a single ```json findings block.',
));
}
let parsed;
try {
parsed = JSON.parse(block);
} catch (e) {
return fail(issue('FINDINGS_PARSE_ERROR', `Reviewer JSON block did not parse: ${e.message}`));
}
return validateFindings(parsed);
}
// ---- CLI shim ----------------------------------------------------------------
if (import.meta.url === `file://${process.argv[1]}`) {
const args = process.argv.slice(2);
const filePath = args.find((a) => !a.startsWith('--'));
if (!filePath) {
process.stderr.write('Usage: findings-schema.mjs [--json] <reviewer-output.txt|.md>\n');
process.exit(2);
}
if (!existsSync(filePath)) {
process.stderr.write(`findings-schema: file not found: ${filePath}\n`);
process.exit(2);
}
const r = validateReviewerOutput(readFileSync(filePath, 'utf-8'));
if (args.includes('--json')) {
process.stdout.write(JSON.stringify({ valid: r.valid, errors: r.errors, warnings: r.warnings }, null, 2) + '\n');
} else {
process.stdout.write(`findings-schema: ${r.valid ? 'PASS' : 'FAIL'} ${filePath}\n`);
for (const e of r.errors) process.stderr.write(` ERROR [${e.code}] ${e.message}\n`);
for (const w of r.warnings) process.stderr.write(` WARN [${w.code}] ${w.message}\n`);
}
process.exit(r.valid ? 0 : 1);
}