fix(reviewer): make deterministic-vs-model disagreement visible - #41
Conversation
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.
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
…ailure 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.
![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: 1618f64b7c
ℹ️ 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".
| feedback=feedback, | ||
| ) | ||
| llm_approved = bool(raw.get("approved", False)) | ||
| feedback = list(raw.get("feedback", []) or []) + list(raw.get("suggestions", []) or []) |
There was a problem hiding this comment.
When the model returns valid JSON but emits feedback or suggestions as a string rather than an array, list(...) silently converts the message into individual characters. In the checked orchestrator.py review path, each character is then printed and joined as a separate issue for replanning, losing the actionable review message; previously this malformed response raised and used the heuristic fallback. Normalize a scalar to one message or reject the invalid shape rather than coercing arbitrary iterables.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Pull request overview
This PR updates the reviewer’s LLM path so that when the deterministic fabricated-precondition check overrides an LLM approval, the resulting feedback explicitly labels the override and preserves visibility of the LLM’s original verdict (instead of silently appending deterministic reasons). It also makes feedback/suggestions merging resilient to null values.
Changes:
- Preserve disagreement visibility by appending a labeled “[deterministic override] … model reviewer APPROVED …” message when deterministic rejection overrides an LLM approval.
- Harden feedback aggregation to handle
nullfeedback/suggestionsfields without errors. - Add tests covering override labeling and the “no override label when both reject” case.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
justai/reviewer.py |
Keeps the LLM verdict visible by explicitly labeling deterministic overrides and safely merging LLM feedback/suggestions with deterministic findings. |
tests/test_reviewer_precondition.py |
Adds regression tests to ensure override labeling/disagreement preservation behavior is enforced. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Held-out eval of the frozen detector (22 independently authored cases) produced 5 real disagreements between the deterministic check and the model reviewer — detector right on 1, model right on 4. PR #40 hid that: it appended the detector's reason without labelling the override or preserving the model's verdict.
Now an override is explicitly labelled and the model's approval stays visible; no label when both reject. Also hardens the merge against null feedback/suggestions.
Evidence:
fable-loop/lab/COMPOSITION-RULE-SPEC.md(pre-registered rule + decision). Focused 16/16; full suite 562 + 14.