mirror of
https://github.com/pbakaus/impeccable.git
synced 2026-09-19 17:46:36 +03:00
Test advice and documentation outcomes rather than reference coverage
Keep completion, consent, documentation evidence, and new-world artifact requirements strict. Record reference-only omissions as diagnostics in the scoped advice and no-change fixtures. AI assistance: Codex, under maintainer direction.
This commit is contained in:
@@ -119,6 +119,32 @@ workflow claim; the reference-loading miss remains visible. No full build was
|
||||
rerun. Focused Claude routing S3/S4, the ordinary suite, source-first build, and
|
||||
generated-skill authoring validation passed. Release #782 remains held.
|
||||
|
||||
### Outcome assertions (maintainer-approved follow-up)
|
||||
|
||||
S16/S17 now require a completed, useful, read-only answer; S17 distinguishes
|
||||
assessment from implementation and explains that critique is optional before
|
||||
polish. Explicit invented prerequisites remain failures. Missing routing or
|
||||
comparison references are TAP diagnostics based on successful content loads,
|
||||
not failed read attempts. These English-fixture phrase checks are bounded
|
||||
regression checks, not a comprehensive semantic grader.
|
||||
|
||||
The resumed ordinary-extension checkpoint accepts a direct documentation pass
|
||||
without the degraded wrapper only with actual document.md, page, and DESIGN.md
|
||||
reads, a concrete no-change report grounded in the fixture's type/palette/layout,
|
||||
and zero mutations. Seeded files must still be byte-identical and pre-existing
|
||||
sidecar drift must remain untouched. New-world documentation writes, redesign
|
||||
ordering, and full-build completion gates are unchanged.
|
||||
|
||||
Offline re-evaluation of saved Claude traces: three advice responses and two
|
||||
evidenced no-op handoffs pass the new assertions. The original post-review
|
||||
baseline and the 637-second full build still fail for absent documentation
|
||||
evidence. Unit negative controls reject fabricated prerequisites, unsolicited
|
||||
edits/interviews/scans, failed reads, empty or unsupported reports, and exhausted
|
||||
budgets. This is assertion replay, not new model evidence or a rerun of cleaned-up
|
||||
workspaces' filesystem checks. No paid calls or skill prose changes were needed.
|
||||
The earlier reference misses above are now diagnostics, not release blockers by
|
||||
themselves; this does not establish an all-green full-workflow matrix.
|
||||
|
||||
Each scenario:
|
||||
|
||||
1. `prepareWorkspace()` uses the production transformer to build current source
|
||||
@@ -241,8 +267,8 @@ results remain the completed measurements.
|
||||
| 13 | empty workspace; prompt is `/impeccable teach` | runs `impeccable context` and diverts into `reference/init.md` because `teach` aliases `init` |
|
||||
| 14 | PRODUCT.md with `## Platform: ios` (native iOS app); prompt is `/impeccable craft a tide detail screen` | `impeccable context` runs and emits the contents of `reference/ios.md` directly, placing native conventions in context without a second model-directed read |
|
||||
| 15 | same iOS fixture; prompt is `/impeccable audit` | agent loads `reference/audit.native.md` (the Commands-table native variant, routed instead of `audit.md`) |
|
||||
| 16 | existing surface, with and without PRODUCT.md; asks where to start | loads `routing.md`, delivers advice, and does not edit project files, start an interview, archive a critique, or run menu scans |
|
||||
| 17 | existing surface; asks whether critique is required before polish | loads `routing.md` and both command references, then delivers advice without executing the playbooks |
|
||||
| 16 | existing surface, with and without PRODUCT.md; asks where to start | completes relevant advice without edits, interviews, critique archives, menu scans, or explicit invented refinement prerequisites; reference coverage is diagnostic |
|
||||
| 17 | existing surface; asks whether critique is required before polish | completes read-only advice distinguishing assessment from implementation and explaining critique is optional; reference coverage is diagnostic |
|
||||
| 18 | existing surface; explicitly requests polish followed by a next-command recommendation | loads `polish.md` rather than substituting workflow advice for the requested work |
|
||||
| 19 | tiny spacing edit with PRODUCT.md + DESIGN.md; Bash denied, a real-loader success control, and a denied-launcher planning-only case | edits require successful playbook/craft-floor reads and a pre-edit denial warning; planning stays read-only and skips craft-floor |
|
||||
|
||||
|
||||
@@ -1,5 +1,48 @@
|
||||
import assert from 'node:assert/strict';
|
||||
|
||||
// These are bounded English-fixture checks, not a general semantic grader.
|
||||
// Reference coverage is reported separately: opening a file proves neither
|
||||
// useful advice nor permission to execute it.
|
||||
export function missingReferences(trace, filenames) {
|
||||
return filenames.filter((filename) => !trace.toolCalls.some((call) =>
|
||||
(call.loadedFiles || []).some((file) => file === filename || file.endsWith(`/${filename}`))));
|
||||
}
|
||||
|
||||
export function assertAdviceOnly(trace, text) {
|
||||
assert.ok(text.trim(), 'advice must reach the user, not stop at reference loading');
|
||||
assert.deepEqual(trace.writePaths, [], 'advice must not use the write tool');
|
||||
const mutations = trace.toolCalls.flatMap((call) => call.mutatedPaths ?? [])
|
||||
.filter((file) => !file.startsWith('.impeccable/') || file.startsWith('.impeccable/critique/'));
|
||||
assert.deepEqual(mutations, [], 'advice must not edit project files or archive an unsolicited critique');
|
||||
assert.deepEqual(trace.questionCalls, [], 'advice must not start an init or design interview');
|
||||
assert.ok(!trace.bashCommands.some((command) => command.includes('impeccable detect')), 'workflow advice does not run menu scans');
|
||||
}
|
||||
|
||||
function normalizedAdvice(text) {
|
||||
return text.replace(/[`*_]/g, '').replace(/[’]/g, "'").toLowerCase();
|
||||
}
|
||||
|
||||
export function assertWorkflowAdvice(trace, text, { missingContext = false } = {}) {
|
||||
assertAdviceOnly(trace, text);
|
||||
const advice = normalizedAdvice(text);
|
||||
assert.match(advice, /index\.html/, 'advice should address the existing surface');
|
||||
assert.match(advice, missingContext ? /\binit\b/ : /\b(?:critique|audit|polish)\b/, 'advice must recommend a relevant starting point');
|
||||
if (missingContext) assert.match(advice, /\bdocument\b/, 'advice should explain how to record the existing identity');
|
||||
assert.doesNotMatch(advice, /(?:must|need to|have to|required to)\s+(?:run\s+)?(?:\/impeccable\s+)?(?:init|document)\b[^.!?\n]{0,100}\bbefore\s+(?:you\s+can\s+)?(?:run(?:ning)?\s+)?(?:polish(?:ing)?|refin(?:e|ing|ement))\b|(?:polish|refinement)\s+(?:requires|is blocked by|cannot run without)\s+(?:init|document|product\.md|design\.md)/,
|
||||
'setup is not a mandatory prerequisite for narrow refinement');
|
||||
}
|
||||
|
||||
export function assertCommandComparison(trace, text) {
|
||||
assertAdviceOnly(trace, text);
|
||||
const advice = normalizedAdvice(text);
|
||||
assert.match(advice, /critique[^.!?\n]{0,120}(?:review|assess|evaluat|report|findings)/, 'comparison must explain critique as assessment');
|
||||
assert.match(advice, /polish[^.!?\n]{0,120}(?:fix|refin|implement|edit)/, 'comparison must explain polish as implementation');
|
||||
assert.match(advice, /critique\s+(?:is\s+)?(?:isn't|is not|not)\s+(?:required|necessary)|critique[^.!?\n]{0,50}\boptional\b|polish[^.!?\n]{0,100}(?:directly|without\s+(?:a\s+)?critique|independent)/,
|
||||
'comparison must explain that critique is optional before polish');
|
||||
assert.doesNotMatch(advice, /(?:must|need to|have to)\s+(?:run\s+)?critique[^.!?\n]{0,80}before\s+(?:run(?:ning)?\s+)?polish|critique\s+(?:is\s+)?(?:required|mandatory|necessary)\s+before\s+polish|polish\s+(?:requires|cannot run without)\s+(?:a\s+)?critique/,
|
||||
'comparison must not invent a critique prerequisite');
|
||||
}
|
||||
|
||||
export function assertNewWorkLifecycle(trace, { target, redesign = false }) {
|
||||
const calls = trace.toolCalls;
|
||||
const writes = (call, file) => (call.mutatedPaths || []).includes(file);
|
||||
|
||||
@@ -28,7 +28,8 @@ import {
|
||||
ENGINE_MISSING_MESSAGE,
|
||||
} from './harness.mjs';
|
||||
import { detectProvider, getModel, hasKey, resolveModelList, PROVIDERS } from './providers.mjs';
|
||||
import { assertPlanningFallbackWarning, LAUNCHER_FAILURE_WARNING } from './assertions.mjs';
|
||||
import { assertPlanningFallbackWarning, LAUNCHER_FAILURE_WARNING, assertAdviceOnly, assertWorkflowAdvice, assertCommandComparison, missingReferences } from './assertions.mjs';
|
||||
import { assertCompleted } from '../skill-workflow/assertions.mjs';
|
||||
import {
|
||||
PRODUCT_MD_SAMPLE,
|
||||
PRODUCT_MD_SAMPLE_NO_REGISTER,
|
||||
@@ -90,16 +91,6 @@ function executedUpdateCommands(trace) {
|
||||
);
|
||||
}
|
||||
|
||||
function assertAdviceOnly(trace, text) {
|
||||
assert.ok(text.trim(), 'advice must reach the user, not stop at reference loading');
|
||||
assert.deepEqual(trace.writePaths, [], 'advice must not use the write tool');
|
||||
const mutations = trace.toolCalls.flatMap((call) => call.mutatedPaths ?? [])
|
||||
.filter((file) => !file.startsWith('.impeccable/') || file.startsWith('.impeccable/critique/'));
|
||||
assert.deepEqual(mutations, [], 'advice must not edit project files or archive an unsolicited critique');
|
||||
assert.deepEqual(trace.questionCalls, [], 'advice must not start an init or design interview');
|
||||
assert.equal(bashCommandsMatching(trace, 'impeccable detect').length, 0, 'workflow advice does not run menu scans');
|
||||
}
|
||||
|
||||
for (const modelId of resolveModelList()) {
|
||||
const provider = detectProvider(modelId);
|
||||
const keyPresent = hasKey(provider);
|
||||
@@ -634,40 +625,42 @@ for (const modelId of resolveModelList()) {
|
||||
['existing project', WORKFLOW_ADVICE_FILES],
|
||||
['missing product context', { 'index.html': MINIMAL_LANDING_HTML }],
|
||||
]) {
|
||||
it(`scenario 16: workflow advice stays read-only (${label})`, async () => {
|
||||
it(`scenario 16: workflow advice stays read-only (${label})`, async (t) => {
|
||||
const workspace = prepareWorkspace({ files });
|
||||
try {
|
||||
const { trace, text } = await runTurn({
|
||||
const result = await runTurn({
|
||||
workspace,
|
||||
model,
|
||||
userPrompt: "I'm joining this project. Where should I start with Impeccable?",
|
||||
maxSteps: 8,
|
||||
contextOnlyBash: true,
|
||||
});
|
||||
const { trace, text } = result;
|
||||
logTrace('S16', label, modelId, trace, { textSample: text.slice(0, 300) });
|
||||
assert.ok(readsMatching(trace, 'reference/routing.md').length, 'workflow advice loads the shared routing reference');
|
||||
assertAdviceOnly(trace, text);
|
||||
t.diagnostic(`Reference coverage gaps (non-blocking): ${missingReferences(trace, ['reference/routing.md']).join(', ') || 'none'}`);
|
||||
assertCompleted(result);
|
||||
assertWorkflowAdvice(trace, text, { missingContext: label === 'missing product context' });
|
||||
} finally {
|
||||
cleanupWorkspace(workspace);
|
||||
}
|
||||
});
|
||||
}
|
||||
|
||||
it('scenario 17: command comparison reads references without running them', async () => {
|
||||
it('scenario 17: command comparison explains independent commands without running them', async (t) => {
|
||||
const workspace = prepareWorkspace({ files: WORKFLOW_ADVICE_FILES });
|
||||
try {
|
||||
const { trace, text } = await runTurn({
|
||||
const result = await runTurn({
|
||||
workspace,
|
||||
model,
|
||||
userPrompt: 'Should I use critique or polish on index.html? Is a critique required before polishing?',
|
||||
maxSteps: 8,
|
||||
contextOnlyBash: true,
|
||||
});
|
||||
const { trace, text } = result;
|
||||
logTrace('S17', 'command-comparison', modelId, trace, { textSample: text.slice(0, 300) });
|
||||
assert.ok(readsMatching(trace, 'reference/routing.md').length, 'a command name in a question still routes to advice');
|
||||
assert.ok(readsMatching(trace, 'reference/critique.md').length, 'comparison consults the critique contract');
|
||||
assert.ok(readsMatching(trace, 'reference/polish.md').length, 'comparison consults the polish contract');
|
||||
assertAdviceOnly(trace, text);
|
||||
t.diagnostic(`Reference coverage gaps (non-blocking): ${missingReferences(trace, ['reference/routing.md', 'reference/critique.md', 'reference/polish.md']).join(', ') || 'none'}`);
|
||||
assertCompleted(result);
|
||||
assertCommandComparison(trace, text);
|
||||
} finally {
|
||||
cleanupWorkspace(workspace);
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user