Repository navigation
chore(review): add additive repository OCR rules - #73
Conversation
lanyue-llk
left a comment
There was a problem hiding this comment.
Reviewed 3a02fcf against the changed rules, README, AGENTS.md, CONTRIBUTING.md, benchmark integration guide and provider contract. The additive configuration matches OCR's resolver implementation. Repository-specific checks below follow the current protocol and contribution guide. I found no configuration defect that warrants blocking this PR. The following changes would make the review process more useful.
-
Make question 4 neutral about implementation style. It already limits speculative abstractions and permits stateless functions, but it still starts by asking whether shared components apply object-oriented principles. Ask whether ownership, state and invariants are clear and whether the existing design is sufficient. Require a class or replaceable interface only when a current caller or lifecycle needs it. A shared pipeline can remain a set of cohesive functions; class adoption alone should not count as an improvement.
-
Add execution to questions 6–7, within the review environment's permissions. They ask whether a reproduction can distinguish failure from a passing control, but never ask the reviewer to run it. Suggested wording: "When execution is available and permitted, run the smallest relevant public-interface reproduction and its passing control; check that reported validation belongs to the reviewed revision." For these rule files, an OCR resolver smoke check on a root file, nested Python file and non-Python file would establish that both the repository text and the appropriate system rule reach the consumer. JSON parsing, question counting and the existing bot review do not establish that: the PR description explicitly says the bot reads target-branch rules.
-
Give native-answer provenance and cohort admission an explicit place in questions 8–9. The provider contract and Web Fetch protocol require the harness to bind selected body, metadata, title, final URL and delivered files to retained native evidence. A decoder can preserve every artifact hash yet score a log message or an undeclared fallback as the answer. Ask for an actual native-envelope-to-scored-answer scenario, and check that incomplete or uncertified cohorts cannot enter the publishable build. Keeping files outside Git does not establish either property.
-
Include the installed integration boundary when relevant. This repository ships Python workers, TypeScript bridges and optional integrations; CONTRIBUTING.md's package gate tests them from source archive and wheel outside the checkout. A checkout-only adapter test can pass while a worker or resource is absent from the distribution. Ask whether changed runtime assets, dependencies and benchmark pins are exercised by that existing gate, including downstream definition/hash compatibility.
These can be short additions or references to the existing owners. There is no need to copy the full track protocols into this supplement. Local structural checks passed for all three PR snapshots; the OCR CLI was not installed, so I checked its published resolver source rather than claiming an executed OCR smoke test.
There was a problem hiding this comment.
✅ Re-review complete · 92d55cc
No findings in this incremental review.
There was a problem hiding this comment.
✅ Re-review complete · 8cb740e
No findings in this incremental review.
Change
Add repository-wide OCR guidance in six titled steps: goals and constraints, regressions, design and reuse, upstream and cross-repository dependencies, validation, and complete coverage. Each step has one question and short action items. Keep object-oriented design and avoidance of report-instance coupling explicit; put repository-specific checks in one short section.
Use
merge_system_rule: trueto preserve OCR's built-in rules, severity thresholds, output format and execution policy. The rules take effect for subsequent reviews after merging into the target branch (main).Validation
git diff --checkpassed.