Run Ralph read-before-edit UI pilot#70
Conversation
Reviewer's GuideIntroduce a Ralph read-before-edit UI-loop pilot that aligns Control Debugger, MCP tools, CLI, and harness exports around the read-before-edit and output-contract scenarios, adds harness state-fidelity probes and verification, and fixes default pattern routing and UI messaging, all covered by MCP/HTTP/CLI/UI tests and documentation updates. Sequence diagram for harness state-fidelity verification via MCP and HTTPsequenceDiagram
actor Operator
participant MCPClient
participant MCPServer
participant VerifyTool as VerifyHarnessStateFidelity
participant Router
participant Adapters as AgentHarnessAdapters
Operator->>MCPClient: call verify_harness_state_fidelity(probe, observed)
MCPClient->>MCPServer: tool invocation
MCPServer->>VerifyTool: execute(params, frame)
VerifyTool->>Adapters: verify_state_fidelity(probe, observed)
Adapters-->>VerifyTool: verification map
VerifyTool-->>MCPServer: reply_json({verification})
MCPServer-->>MCPClient: MCP result
MCPClient-->>Operator: show verification.status
Operator->>Router: POST /v1/policy-authoring/harness-adapters/state-fidelity/verify
Router->>Adapters: verify_state_fidelity(probe, observed)
Adapters-->>Router: verification map
Router-->>Operator: {"verification": verification}
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces the "Ralph Loop Acceptance Criteria" to ensure functional symmetry across UI, MCP, and CLI surfaces, supported by a detailed pilot run report for tool misuse scenarios. Key technical changes include the implementation of a mapping between counterfactual examples and default workbench patterns, updates to the Lustre Control Debugger model to synchronize these patterns during example selection and transcript loading, and the addition of corresponding regression tests. Feedback was provided regarding the need to consistently update the pattern ID in both the success and failure branches of the transcript loading logic to ensure the UI state remains synchronized.
| pattern_id: external_default_pattern_id_for_example( | ||
| model.example_id, | ||
| ), |
There was a problem hiding this comment.
The pattern_id is explicitly updated here when the transcript loads successfully, but it is not updated in the failure branch (line 327). While ExampleChanged handles the initial sync, if the intent of this fix is to ensure the pattern picker is reset to the example's default after a recording run, it should be applied consistently regardless of whether the transcript loading succeeded, as long as the recording itself was successful.
There was a problem hiding this comment.
Pull request overview
Adds a first “Ralph UI-loop” pilot report for the read-before-edit tool-misuse scenario while fixing a Control Debugger UX mismatch where recording/selecting certain counterfactual examples defaulted to the wrong Workbench pattern. This helps keep Control Debugger evidence capture aligned with the correct policy/workbench slice (tool governance vs output contract review), and documents expectations for UI/MCP/CLI symmetry in future runs.
Changes:
- Document a full Ralph run (steps, evidence, screenshots list, and follow-ups) under
docs/ralph-runs/2026-05-22-read-before-edit-tool-misuse/. - Update the Lustre Control Debugger to pick a default Workbench pattern based on the selected/recorded counterfactual example.
- Add regression tests asserting the example→pattern mapping (read-before-edit → tool-governance, output-contract → ambiguous-success).
Reviewed changes
Copilot reviewed 13 out of 24 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| docs/ralph-runs/2026-05-22-read-before-edit-tool-misuse/ui-pilot-final-evidence.txt | Captures final UI evidence text for the pilot after the mapping fix. |
| docs/ralph-runs/2026-05-22-read-before-edit-tool-misuse/ui-pilot-before-change-evidence.json | Structured “before change” evidence index + text samples for screenshots. |
| docs/ralph-runs/2026-05-22-read-before-edit-tool-misuse/ui-friction-default-pattern-before-change.txt | Evidence of the prior default-pattern mismatch in the UI. |
| docs/ralph-runs/2026-05-22-read-before-edit-tool-misuse/README.md | Narrative pilot report: scenario, repro path, observed failure, and outcomes. |
| docs/ralph-runs/2026-05-22-read-before-edit-tool-misuse/followups.yml | Follow-up work items derived from the pilot (UI clarity, capability gaps, reliability, test gap). |
| docs/ralph-runs/2026-05-22-read-before-edit-tool-misuse/evidence.md | Run commands, URLs, screenshots list, receipts/transcripts IDs, and validation results. |
| docs/agent-control-debugger.md | Adds Ralph loop acceptance criteria focused on UI/MCP/CLI parity and “cold start” usability. |
| docs/agent-authoring.md | Documents wardwright tools as the cold-start entrypoint and reiterates UI/MCP/CLI symmetry expectations. |
| app/test/workbench_test.exs | Adds a regression test that exercises the example→pattern targeting behavior. |
| app/src/wardwright/lustre_control_debugger.gleam | Uses an example-aware default-pattern function during init, example changes, and example recording. |
| app/src/wardwright/lustre_control_debugger_test_support.gleam | Adds test helpers to assert the selected Workbench pattern after recording examples. |
| app/lib/wardwright/cli.ex | Clarifies CLI help text expectations for wardwright tools as an agent-facing discovery surface. |
| app/lib/wardwright_web/control_debugger_data.ex | Introduces default_pattern_id_for_example/1 mapping counterfactual examples to the appropriate Workbench pattern id. |
| def default_pattern_id_for_example("read-before-edit"), do: "tool-governance" | ||
| def default_pattern_id_for_example("output-contract"), do: "ambiguous-success" | ||
| def default_pattern_id_for_example(_example_id), do: default_pattern_id() |
|
Ralph loop continuation update:
Adversarial review: the main tradeoff is losing raw screenshots and JSON captures from earlier Ralph loops. I think that is the right call: the artifacts were large and review-hostile, while the retained tracker plus commits/tests preserve the actionable evidence. Open risk remains |
|
Ralph loop 5 update:
Validation:
Adversarial review: this does not close RALPH-RBE-002. It deliberately makes that proof gap harder to miss by exposing the verification requirement and next-step checklist wherever an adapter/export is consumed. |
|
Ralph loop 6 update:
Validation:
Adversarial review: this still does not close RALPH-RBE-002. The probe makes the future state-fidelity trial falsifiable, and the RouterCase isolation fix removes test pollution without changing production transcript behavior. |
|
Ralph loop 7 update:
Validation:
Adversarial review: this moves RALPH-RBE-002 from an exported probe to a falsifiable verification step, but it intentionally does not close RBE-002. I also caught and fixed a stale docs anchor before push. |
|
Ralph loop 8 update:
Validation:
Adversarial review: this does not close RALPH-RBE-002. It only removes a usability gap between export and verification. I caught and fixed one wording issue before push: the first draft said |
|
Focused RBE-002 loop update pushed in Result: OpenCode import/fork works as best-effort evidence handoff, but does not preserve Wardwright trace/tool results as native OpenCode tool state. What I verified:
Conclusion: keep PR #70's conservative fidelity warnings. The handoff is useful for human/model continuation, but it is not equivalent agent resume. Local validation before push: full |
|
@sourcery-ai review |
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey - I've found 2 issues, and left some high level feedback:
- The new
verify_state_fidelity/2path inAgentHarnessAdaptersis doing several distinct things (event extraction, fingerprinting, comparison, status assembly); consider extracting the event-normalization and diffing logic into smaller private helpers or a separate module to keep the main verifier easier to follow and reuse. ControlDebuggerToolsnow reimplements several concepts already present inControlDebuggerData(event labels, details, fork recommendations, suggested cursors); it may be worth centralizing this summarization logic in one place to avoid future drift between UI and MCP/API behavior.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The new `verify_state_fidelity/2` path in `AgentHarnessAdapters` is doing several distinct things (event extraction, fingerprinting, comparison, status assembly); consider extracting the event-normalization and diffing logic into smaller private helpers or a separate module to keep the main verifier easier to follow and reuse.
- `ControlDebuggerTools` now reimplements several concepts already present in `ControlDebuggerData` (event labels, details, fork recommendations, suggested cursors); it may be worth centralizing this summarization logic in one place to avoid future drift between UI and MCP/API behavior.
## Individual Comments
### Comment 1
<location path="app/lib/wardwright_web/control_debugger_tools.ex" line_range="318" />
<code_context>
+
+ defp event_detail(_event), do: "recorded event"
+
+ defp compact_json(value), do: value |> JSON.encode!() |> String.slice(0, 180)
+
+ defp overlay_for_example(example_id) do
</code_context>
<issue_to_address>
**issue (bug_risk):** Consider making compact_json resilient to JSON encoding failures
compact_json/1 calls JSON.encode!/1 directly, so any non-encodable value in an event (e.g., structs or unknown types) will raise and crash the endpoint. Since these values come from runtime traces, it’d be safer to follow the safe_json pattern (encode with a rescue that falls back to inspect/1) so we return a structured error instead of failing the request.
</issue_to_address>
### Comment 2
<location path="app/lib/wardwright_web/control_debugger_tools.ex" line_range="199" />
<code_context>
+ end
+ end
+
+ defp selected_trace_events(_events, _cursor), do: {:error, "session_id and trace_cursor are required"}
+
+ defp scenario_from_trace(pattern_id, session_id, cursor, selected, events, params) do
</code_context>
<issue_to_address>
**nitpick:** Error message in selected_trace_events mentions session_id even though it only receives events and cursor
Since this function only takes `events` and `cursor`, the current `{:error, "session_id and trace_cursor are required"}` message can be misleading when it bubbles up via `save_evidence`. The actual precondition here is just a non-blank cursor. Please update the message to something like `"trace_cursor is required"` and let higher-level code add any session_id context if needed.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
|
|
||
| defp event_detail(_event), do: "recorded event" | ||
|
|
||
| defp compact_json(value), do: value |> JSON.encode!() |> String.slice(0, 180) |
There was a problem hiding this comment.
issue (bug_risk): Consider making compact_json resilient to JSON encoding failures
compact_json/1 calls JSON.encode!/1 directly, so any non-encodable value in an event (e.g., structs or unknown types) will raise and crash the endpoint. Since these values come from runtime traces, it’d be safer to follow the safe_json pattern (encode with a rescue that falls back to inspect/1) so we return a structured error instead of failing the request.
| end | ||
| end | ||
|
|
||
| defp selected_trace_events(_events, _cursor), do: {:error, "session_id and trace_cursor are required"} |
There was a problem hiding this comment.
nitpick: Error message in selected_trace_events mentions session_id even though it only receives events and cursor
Since this function only takes events and cursor, the current {:error, "session_id and trace_cursor are required"} message can be misleading when it bubbles up via save_evidence. The actual precondition here is just a non-blank cursor. Please update the message to something like "trace_cursor is required" and let higher-level code add any session_id context if needed.
|
UI follow-up pushed in 908a469. This applies the blind design-review pass directly to this PR: clearer example-vs-live-model framing, simpler labels, a concise latest-run summary, technical rule names hidden behind details, and updated browser smoke coverage for the renamed registered/live model surface. Validation: mise check passed locally and in the push hook. |
|
Updated the Ralph/Lustre branch through 58e14d5. The legacy /policies LiveView workbench code and assets are removed; /policies now only redirects to /admin, preserving model links. The Lustre admin now carries the example model library, and the browser smoke target is the Lustre workbench. Local full mise check and pre-push checks passed; GitHub checks are still running. |
Spike Pi and OMP replay exports
Summary
Validation
Adversarial Review
Summary by Sourcery
Strengthen the read-before-edit control debugger loop by exposing it consistently across UI, MCP, CLI, and protected HTTP APIs, and by formalizing harness state-fidelity probing and verification without over-claiming equivalent agent resume.
New Features:
wardwright tools.Bug Fixes:
Enhancements:
wardwright toolsas a cold-start entrypoint and document environment-based storage options and MCP/CLI/UI symmetry expectations.Tests: