fable-implementation-review.md

Fable implementation snapshot review

Verbatim model response; findings refer to the reviewed earlier source snapshot. See reviews.md for final dispositions.

Review of the supplied source only. I did not run anything, and I am not assuming any tests pass or that any recorded evidence exists.

Blockers

1. Evidence directory reuse contaminates the audit trail. In src/evidence.ts, the Evidence constructor accepts a non-empty directory and emit appends to the existing events file. The default replay artifact path encourages a fixed discovery directory. Repro: run discover twice with the same --out. The events file then holds two run ids, stale screenshots from the longer run survive, and finish hashes them into a manifest claiming the second run id. Smallest fix, in the constructor after the directory is created:

   if(readdirSync(directory).length)throw new RunError('evidence_dir_not_empty');

2. Discovery can record steps that can never replay. In src/discovery.ts, discover records a decision as soon as act succeeds and recordStep in src/contract.ts assigns postconditions by label alone. Nothing verifies the postcondition at record time. Repro: the model clicks Review invoice before filling fields. The fixture returns a 400 form with a validation message that condition does not classify. Discovery continues, the final checkpoint passes, and the artifact contains a click step tagged review_or_outcome mid-sequence. Every replay then fails at that step with checkpoint_mismatch. Fix, after the act in the discovery loop:

   const recorded=recordStep(decision);
   await surface.verifyStep(recorded); // throws checkpoint_mismatch: do not record, do not finish

   A zero-LLM dry replay before saving would be stronger, but the one-line verification closes the hole.

3. Human action capture has blind spots and there is no abort channel. In src/surface.ts, the binding installed in open only emits when the owner is exactly human. Repro: run replay with --human --scenario modal. In the LedgerDesk window, click Dismiss notice before pressing Claim. Then Claim and Resume form. The run succeeds and the events file contains a human resume with zero operator actions. The same gap covers anyone typing into the form while automation owns the session. Also, in src/control.ts, resume never emits the resuming state and silently rolls back to human on a failed verify. Fix: record every UI event with attribution and expose the in-flight flag.

   // control.ts
   get busy(){return this.flight!==null;}
   // surface.ts binding
   options.evidence?.emit({event:'ui_event',owner:surface.control.owner,automationBusy:surface.control.busy,detail:event});

   An event with owner automation and busy false is a human acting outside ownership and should be flagged. Emit ownership on entering resuming and on rollback. Keeping the console alive for the whole human run with an abort action that calls terminate is the smallest way to give the operator a sanctioned stop.

4. The persisted result contradicts the output contract. outputContract declares invoice number as a plain string, but Evidence.save in src/evidence.ts runs input-value redaction over the result. Repro: any successful replay. The result file shows the literal placeholder text instead of the invoice number, and stdout never prints outputs at all. The only durable record of what the checkpoint observed is therefore tautological. Pick one: either declare the output redacted in the contract, or exempt the result file from input redaction while keeping the credential scrub:

   let safe=name==='result.json'?redact(value):redact(value,this.inputs);

5. Click targets resolve to non-interactive text, producing a phantom second action. In src/surface.ts, resolve for text targets uses a bare frame-scoped text match. Repro: replay with --human --scenario modal. Step 0 navigates to the form, the modal is seen, and replay in src/replay.ts hits continue without advancing. After the operator resumes, step 0 runs again and matches the New Invoice heading, clicks it, and passes the form_ready check. The evidence shows two action events for step 0 and an intervention claiming the step was incomplete. On a target with a different heading the retry would fail with target_missing. Two small corrections:

   // surface.ts resolve, click targets only
   locator=frame.getByText(target.text,{exact:true}).locator('xpath=ancestor-or-self::*[self::a or self::button][1]');
   // replay.ts after act
   if(await surface.condition()==='unexpected_modal'){if(step.postcondition==='form_ready')index++;continue;}

6. Substring redaction corrupts evidence and the model's own observations. redact in src/policy.ts splits on raw vendor and invoice values with no word boundary, and the schema allows one-character values. Repro: vendor "A". Every capital A in observations, prompts, step reasons and events becomes the vendor placeholder, so the model reads mangled page text during discovery. Fix: raise the minimums in InputsSchema to three characters and use a bounded regex:

   s=s.replace(new RegExp(`\\b${raw.replace(/[.*+?^${}()|[\]\\]/g,'\\$&')}\\b`,'g'),`{${key}}`);

7. A recoverable outcome after the last step is misreported as a hard checkpoint failure. verifyStep in src/surface.ts returns early on any condition, and the post-loop code in replay only maps the two business codes before calling checkpoint, whose own guard throws checkpoint_mismatch. Repro: a schema-valid artifact whose final step is the New Invoice click, run under --scenario notice. The session notice is visible after the step, the recovery budget is never applied, and the result is a hard checkpoint_mismatch. Fix after the loop:

   if(condition)throw new RunError(condition);

   and have the guard inside checkpoint throw the condition code rather than checkpoint_mismatch.

8. The session-notice rule is never exercised by discovery. discover in src/discovery.ts throws on any condition, including session_notice, so Continue session is unreachable during discovery and the rule listed in the artifact is hardcoded rather than observed. Repro: discover with --scenario notice fails right after the New Invoice click. Fix: mirror the replay recovery inside the discovery loop, with the same budget of two, without recording the click as a step.

Limitations to document, not blockers

• Target mode is fixture-only in practice. Surface.open requires the LedgerDesk 1.0 header text on the top page and screenshot masking relies on a fixture-specific class. State plainly that --target is untested against anything else.
• The artifact schema is all literals. Only steps and provenance vary. Say this is a single-capability proof, not a general artifact format.
• A policy-denied model decision ends discovery. There is no feedback loop to the model. Note it as a hard outcome by design.
• Stdout mixes the operator link with the JSON result. Consumers parsing stdout will break during human runs.
• Early failures lack a run header. The run_started event is emitted after the surface opens, so incompatible_surface and policy failures at open have a result file but no header. The manifest is also skipped if close rejects, since finish is not in its own try block.
• Type drift. The cli catch block writes a failure result without the step, expected and observed fields the Result type requires. RecordedStepSchema spreads the step shape and drops the click-has-no-value refinement, so hand-edited artifacts can pair a fill with a form_ready postcondition and skip value verification until the final checkpoint.
• Date and category are not redacted, and the select is not masked in screenshots. Low sensitivity, but say so.
• Unverifiable from source. I cannot confirm that the Claude CLI accepts --safe-mode, --effort, or the model alias passed by ClaudeClient, nor that the loose fable substring check on usage identifies the intended model. Do not claim a live discovery run happened unless the events file and manifest from that run are in the submission.