Skip to content

Commit fb77315

Browse files
authored
Merge pull request #4998 from loopx-project/codex/pr-review-evidence-depth-0924
Review sustained progress and user experience before PR approval
2 parents 2e1e632 + e363b96 commit fb77315

17 files changed

Lines changed: 1454 additions & 116 deletions
Lines changed: 65 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,65 @@
1+
# Reviews grounded in historical code
2+
3+
These public historical reviews exercise the shared publication/readback
4+
validator and the opt-in review decision probes. They replace the invented
5+
export example as the default body fixture. They are not new approvals,
6+
merge permissions, or evidence that today's checkout passes historical tests.
7+
8+
| Review | Why retain it | Code boundary |
9+
| --- | --- | --- |
10+
| [#4854 request changes](https://github.com/loopx-project/loopx/pull/4854#pullrequestreview-5287932132) | A concrete retry counterexample overturns an earlier approval at the same head; the repair preserves useful replay assets while removing duplicate fixture authority. | `external_progress_review.py` drops an earlier typed claim when deduplicating Turns; `replan_semantics.ts` relies on that window to refuse replay. |
11+
| [#4882 request changes](https://github.com/loopx-project/loopx/pull/4882#pullrequestreview-5277672546) | Passing receipt and lock tests did not exercise the actual writer. The review spells out an interleaving and asks for the correct owner to enforce freshness. | `checkpoint_context_io.py` holds projection/source locks, while `provider_update.py` commits canonical state before projection settlement. |
12+
| [#4882 approval after repair](https://github.com/loopx-project/loopx/pull/4882#pullrequestreview-5288050653) | A positive control: accept a demonstrated local fix while disclosing uncertain append recovery and excluding PostgreSQL. | `checkpoint_authority.ts` and `checkpoint_commit.ts` keep comparison and append inside the File writer lock or SQLite transaction. |
13+
14+
The Markdown bodies preserve the public reviews. `../pr-review.body.md` is
15+
the last review with only the exact head and English verdict replaced by
16+
`HEAD_OID` / `VERDICT` for queue and parser tests; substitutions in those tests
17+
are mechanical mutations, not endorsements of another head or verdict. Other
18+
bodies only normalize trailing whitespace. `cases.json` records original
19+
review URLs, exact commits, normalization and original response-body digests.
20+
Code excerpts carry immutable source URLs, file paths and inclusive line
21+
ranges. Each `lines` array is an exact contiguous slice, including newlines,
22+
of the file at that commit. Inspect the linked full file for omitted context.
23+
24+
Selection is based on causal analysis, concrete symbols, negative cases and
25+
bounded conclusions, not size or approval state. The long [#4683
26+
review](https://github.com/loopx-project/loopx/pull/4683#pullrequestreview-5243962661)
27+
still missed enabled-but-out-of-scope acceptance interference, later repaired
28+
in #4989. Its length must not become a quality oracle. The acceptance-scope
29+
negative/positive probes remain in `test_pr_review_behavior.py`.
30+
31+
Two different checks consume this corpus:
32+
33+
- Body tests read the historical Markdown to ensure detailed real reviews
34+
satisfy the format contract. `evidence_truth_verified` remains false.
35+
- Opt-in model probes receive only `scenario`: actual source excerpts, the
36+
accepted outcome and explicitly historical observations. The review body,
37+
published conclusion, `expected_verdict` and `decisive_location` are withheld. The checkpoint
38+
before/after pair guards against blanket rejection of locks or SQLite.
39+
40+
The historical probes must also locate the decisive source range. A correct
41+
rejection blaming the wrong owner fails this check. For #4854 the excerpts
42+
include the Python window producer, novelty codec and TypeScript consumer;
43+
omitting the codec would invite an unsupported claim that TypeScript should
44+
recompute novelty. Review the saved explanation as well: a matching location
45+
and verdict still cannot mechanically certify the reasoning.
46+
47+
Expected decisions come from the stated invariant and inspected source, not
48+
automatically from a historical approval. For retry claims, an omitted earlier
49+
claim cannot become new evidence. For checkpointing, every canonical writer
50+
must share the fence through append. A fixed local boundary can be accepted
51+
without pretending external files participate in SQLite rollback.
52+
53+
Run from the checkout root:
54+
55+
```sh
56+
uv run --extra test python -m pytest tests/capabilities/test_pr_review_body.py -q
57+
# Requires the normal process-only provider credentials; no tools or writes.
58+
LOOPX_REVIEW_LIVE_TEST=1 uv run --extra test python -m pytest \
59+
tests/capabilities/test_pr_review_behavior.py -k historical -q
60+
```
61+
62+
Live results qualify reasoning over supplied evidence only. They do not
63+
demonstrate autonomous repository investigation, rerun the old provider
64+
concurrency tests, or establish an improvement over a baseline model. No
65+
private Goal state or raw execution logs belong in these fixtures.

0 commit comments

Comments
 (0)