Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
4 changes: 3 additions & 1 deletion DOCS.md
Original file line number Diff line number Diff line change
Expand Up @@ -1466,7 +1466,9 @@ as disproportionate.
components from the registry, the shared DataTable for record lists,
semantic components for known-domain fields — with `ai-docs/ui/patterns.md`
overriding the defaults when the project keeps one. Violations get ONE
`fix_ui` builder round, `ui_verify` re-audits, the final `ui_gate` (code)
`fix_ui` builder round; `ui_verify` then re-audits ONLY the violations the
first audit found (the full rubric was already answered — re-sending it
re-pays the reviewer for settled questions); the final `ui_gate` (code)
throws if violations survive, and `ui_retest` re-runs the suite (the
repair touched production code after the test phase). The rubric lives in
`modules/` (not prompt material), so `--update-runtime` delivers it to
Expand Down
12 changes: 6 additions & 6 deletions fia-templates/data/prompt_engineering/reviewer/system.md
Original file line number Diff line number Diff line change
Expand Up @@ -11,11 +11,11 @@ Verify the implementation matches the request. Read-only with respect to product
file on disk and audit its checkboxes: every `[x]` is a claim — one the
diff does not support is grounds for rejection, and remaining `- [ ]`
items are unfinished work.
- When the diff touches frontend component files, also audit them against
`ai-docs/ui/patterns.md` (when present) and its defaults: field errors
inline with the field (never only a banner/toast), success/failure toasts
after mutations resolve, create/edit in a `Dialog`, `AlertDialog` for
destructive actions, components from `ai-docs/components/registry.md`.
A violation is grounds for rejection like any other unmet requirement.
- UI conformance is owned by the dedicated UI gate: when your task prompt IS
a UI rubric (it says so explicitly), audit exactly that rubric. In a general
review, do NOT re-audit `ai-docs/ui/patterns.md` — the run's UI gate phases
already settled conformance before you, and re-auditing pays the same work
twice. A UI defect that breaks the ASK itself still counts, like any other
unmet requirement.
- Set `approved` honestly; list blocking items if not approved.
- Emit ONLY valid JSON matching ReviewOutput.
6 changes: 5 additions & 1 deletion fia-templates/data/prompt_engineering/reviewer/user.md
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,11 @@

## Task

Review whether what was built satisfies `prompt`. Use git diff and the codebase.
Review whether what was built satisfies `prompt`. Your scope is the DIFF:
`git diff`, plus the files named in `previous_envelope` (`changed_files` /
`artifacts`). Read other code only when a changed file forces you to — its
direct callers, the contract it implements — and never crawl the repository
looking for unrelated problems: an issue outside the diff is out of scope.

If `prompt` is an implementation brief, also read its CURRENT file in
`ai-docs/actual-todo/` and audit the checkboxes: a `[x]` the diff does not
Expand Down
27 changes: 24 additions & 3 deletions fia-templates/modules/ui-gate.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -169,6 +169,28 @@ function checkPrompt(files, rulePlan, contract) {
].join('\n');
}

/**
* The re-audit after the repair round is scoped to the VIOLATIONS the first
* audit found — that audit already covered the full rubric and its other
* items passed; re-sending the whole rubric re-pays the reviewer for
* questions that were already answered (measured: the verify pass was the
* single largest UI-reviewer line on a real project).
*/
function verifyPrompt(files, problems) {
return [
'Re-audit ONLY the violations listed below. The first audit already covered the full UI rubric and every other item passed — do not re-judge them and do not raise new findings outside this list.',
'The UI contract (`ai-docs/ui/contract.json`) and the project catalogs still govern any violation that cites them.',
'',
'Files touched by the repair round:',
...files.map((f) => `- ${f}`),
'',
'Violations to verify — emit ONE finding per item ({requirement, met, evidence}, evidence citing file/line):',
...problems.map((p, i) => `${i + 1}. ${p}`),
'',
'Set approved=true ONLY when every violation above is fixed; otherwise list exactly what still fails in `blocking`.',
].join('\n');
}

/**
* Deterministic UI-conformance close-out, same shape as the checklist gate:
* audit → one builder repair round when violations were found → re-audit,
Expand Down Expand Up @@ -286,10 +308,9 @@ export async function runUiGate(run, prompt) {
async (ph) =>
ph.call({
outputType: 'ReviewOutput',
prompt: checkPrompt(
prompt: verifyPrompt(
[...new Set([...scope.uiFiles, ...(fix.changed_files || []).filter((f) => FRONTEND_FILE.test(f))])],
scope.rulePlan,
scope.contract,
problems,
),
previous: fix,
gates: [verdictConsistent],
Expand Down
47 changes: 47 additions & 0 deletions test/ui-gate.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -395,3 +395,50 @@ test('FRONTEND_FILE: components and presentation styles arm the gate', () => {
assert.ok(!FRONTEND_FILE.test(f), `${f} should NOT count as frontend`);
}
});

// ── verify pass scoped to the violations ─────────────────────────────────────

test('ui_verify re-audits ONLY the violations, never the whole rubric again', async () => {
const repo = uiFixture({ testExit: 0 });
const calls = {};
const run = fakeUiRun(repo, {
onCall: (name, call) => {
calls[name] = call;
if (name === 'ui_check')
return {
approved: false,
blocking: ['field errors render in a banner'],
findings: [{ requirement: 'toasts after mutations', met: false, evidence: 'app/Form.tsx:12 fires before resolve' }],
};
if (name === 'fix_ui') return { status: 'success', changed_files: ['app/Fixed.tsx'], artifacts: [] };
if (name === 'ui_verify') return { approved: true, findings: [] };
throw new Error(`unexpected agent phase ${name}`);
},
});
await runUiGate(run, UI_BRIEF);
// The first audit carries the full applicable rubric…
assert.match(calls.ui_check.prompt, /Applicable rubric/);
// …the verify pass carries only the delta: the violations and the touched files.
assert.match(calls.ui_verify.prompt, /Re-audit ONLY the violations/);
assert.match(calls.ui_verify.prompt, /field errors render in a banner/);
assert.match(calls.ui_verify.prompt, /toasts after mutations/);
assert.match(calls.ui_verify.prompt, /app\/Fixed\.tsx/);
assert.ok(!calls.ui_verify.prompt.includes('Applicable rubric'), 'the full rubric must not be re-sent');
assert.ok(
!calls.ui_verify.prompt.includes('Deterministic contract decisions'),
'the applicability table was already settled by the first audit',
);
assert.match(calls.ui_verify.prompt, /do not raise new findings outside this list/);
});

// ── reviewer prompts are diff-scoped and never re-audit the UI gate's work ───

test('reviewer prompts: scope is the diff, and the general review does not re-audit patterns.md', () => {
const system = readFileSync(new URL('../fia-templates/data/prompt_engineering/reviewer/system.md', import.meta.url), 'utf8');
const user = readFileSync(new URL('../fia-templates/data/prompt_engineering/reviewer/user.md', import.meta.url), 'utf8');
assert.match(user, /Your scope is the DIFF/);
assert.match(user, /never crawl the repository/);
assert.ok(!user.includes('Use git diff and the codebase.'), 'the repo-wide instruction must be gone');
assert.match(system, /do NOT re-audit `ai-docs\/ui\/patterns\.md`/);
assert.match(system, /UI conformance is owned by the dedicated UI gate/);
});
Loading