fix(reviewer): fabrication check backstops the live LLM path - #40
Conversation
…allback review() called the LLM first and only used _heuristic_review (which holds the fabricated-precondition check) when the LLM failed -- so in normal operation the deterministic backstop never ran, and an LLM that approved a plan manufacturing a required input would pass it through. Now the fabricated-precondition check runs on every path and overrides an LLM approval; the detector (and its false-positive guards) is unchanged from #39. Tests: review() rejects a fabrication the mocked LLM approves; review() keeps a clean plan approved. Focused 14/14; full suite 560 + 14 subtests.
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
There was a problem hiding this comment.
Pull request overview
This PR ensures the deterministic “fabricated precondition” detector is enforced on the primary production path (LLM-first review()), so an LLM approval cannot allow a plan that manufactures goal-assumed inputs.
Changes:
- Run
_fabricated_preconditions(plan)on everyreview()call (not only in the heuristic fallback). - Override LLM approval when fabrication is detected and append deterministic findings to returned feedback.
- Add tests covering (1) fabrication rejection even when the LLM approves, and (2) preserving LLM approval for a clean plan.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
justai/reviewer.py |
Adds a deterministic fabrication backstop to the LLM-first review() path and ensures it can override LLM approval. |
tests/test_reviewer_precondition.py |
Adds regression tests confirming the fabrication backstop applies to review() and does not block clean plans. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: cc16209297
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| # high-precision, so run it on EVERY path -- an LLM that approves a plan | ||
| # which manufactures a required input must still be overridden. (Otherwise | ||
| # the check only ran on LLM failure, i.e. never in normal operation.) | ||
| fabricated = _fabricated_preconditions(plan) |
There was a problem hiding this comment.
When a valid summarization task is titled Generate report.csv summary, _task_makes_exist matches generate report.csv against the input basename even though report.csv modifies the following output noun; the same occurs with titles such as Create report.csv backup. Applying this detector to every live review now turns an otherwise valid LLM approval into a rejection, potentially exhausting the replan loop, so the prose match should distinguish creation of the input itself from creation of an artifact derived from it before serving as a mandatory backstop.
Useful? React with 👍 / 👎.
* fix(reviewer): make deterministic-vs-model disagreement visible A held-out evaluation of the frozen detector produced 5 real disagreements between the deterministic precondition check and the model reviewer (each was right where the other was wrong). PR #40 appended the detector's reason but did not say an override had occurred or keep the model's verdict visible, so an operator could not tell the two reviewers disagreed. - When the model approves and the check rejects, feedback now carries an explicit '[deterministic override]' line naming that the model approved. - No override line when both reject (no disagreement to report). - Also hardens the feedback merge against a null feedback/suggestions field. Focused 16/16; full suite 562 + 14 subtests. * review fixes: coerce feedback fields; always surface model-reviewer failure Address independent review of PR #41: - _as_feedback_list: a model returning a bare string ('plan is fine') was shredded into single-character items by list(); previously that malformed shape raised and fell back loudly, so this was a regression. Now coerced. - The '[heuristic review: ...]' marker was skipped when the heuristic produced no feedback, so a model-reviewer outage read as a clean full review. Now always emitted, carries the error message, and rebinds instead of mutating (a non-list field can no longer throw inside the handler and lose the error). Focused 18/18; full suite 564 + 14 subtests.
…48) The installed-executor productive trial re-ran with #47 installed and the vacuous criterion was still approved: the check lived only in _heuristic_review, which runs solely when the model call fails. In normal operation the model reviewer approved grep -Fxq 'Hello from JustAi' greeting.py && echo 'success' || echo 'failure' so the task verified vacuously and the receipt again read 'done: failure'. Same shape as #40 -- a deterministic backstop is worthless unless it runs on the path production actually takes. Focused 24/24; full suite 607 + 14 subtests.
The fabricated-precondition check only ran in the offline heuristic fallback, so production review() (LLM-first) bypassed it. Now it runs on every path and overrides an LLM approval. Detector unchanged from #39 (its FP guards still apply). Tests: rejects a fabrication the LLM approves; keeps a clean plan approved. Full suite 560 + 14.