From 3a02fcfe6ac3dcb27273d277f97e231ec30b4472 Mon Sep 17 00:00:00 2001 From: lanyue-llk <270302213+lanyue-llk@users.noreply.github.com> Date: Tue, 27 Oct 2026 17:44:24 +0800 Subject: [PATCH 1/4] chore(review): add additive repository OCR rules --- .opencodereview/review-rules.md | 21 +++++++++++++++++++++ .opencodereview/rule.json | 9 +++++++++ 2 files changed, 30 insertions(+) create mode 100644 .opencodereview/review-rules.md create mode 100644 .opencodereview/rule.json diff --git a/.opencodereview/review-rules.md b/.opencodereview/review-rules.md new file mode 100644 index 000000000..ada02ba76 --- /dev/null +++ b/.opencodereview/review-rules.md @@ -0,0 +1,21 @@ +# Repository review supplement + +These questions supplement OCR's built-in review instructions. Preserve its scope, severity thresholds, output format and execution policy; where they conflict, follow the built-in instructions. Use the questions to investigate the change, not as a checklist to repeat in the review output. Apply the repository's review-relevant constraints in `AGENTS.md` and `CONTRIBUTING.md`. + +Follow this review path, checking each answer against the current diff, relevant callers and evidence: + +1. What user problem and acceptance criteria does this PR address? Does the proposed approach achieve them, and do the implementation and every changed file serve that purpose? Which claims in the description or earlier comments need independent verification? +2. How does the change behave through the supported public entry points and downstream consumers? Could it break previously correct behavior or introduce a new defect through defaults, old inputs, boundary cases or interactions outside the diff? If it restores a removed behavior, does Git history explain why it was removed and whether that reason still applies? +3. What happens with invalid or missing input, partial failure, timeouts, retries, cancellation and concurrent work where relevant? Can errors be swallowed, required checks bypassed, state corrupted, resources leaked or secrets/private data exposed? +4. Does the design apply object-oriented principles to shared components: clear responsibilities, encapsulated state and invariants, and stable interfaces that let implementations vary without changing callers? Does it reuse existing objects and upstream capabilities, favor composition over inheritance, and keep shared behavior under one owner? Could scattered conditionals, exposed internals or duplicated pipelines be replaced by a cohesive object or interface? Is the abstraction justified by current behavior, with simple stateless transformations kept as functions and no unnecessary class hierarchies? +5. Would another supported dataset, provider, cohort or report use the same implementation through explicit data or configuration? Is reusable code coupled to one report's IDs, paths, participant count, measurements or prose, and would adding another instance require edits across unrelated modules? +6. Do tests exercise the intended behavior through supported interfaces, including valid and invalid scenarios and the previously correct behavior at risk? Do fixtures reflect real producer/consumer contracts? Can a focused reproduction distinguish the defect from its passing control, rather than merely confirm the current implementation? Do validation results actually cover the changed behavior? +7. Have all changed files and affected shared paths been examined, including other occurrences of a suspected root cause? Where the review environment supports agents, would independent parallel checks of distinct risk areas improve coverage within OCR's execution policy? Before finishing, have candidate findings been verified and duplicates combined, including earlier findings that may still reproduce on the current revision? + +Report actionable findings under OCR's normal reporting rules, supported by a concrete trigger, affected behavior and code evidence. Treat design or optimization concerns as findings only when they have a demonstrable consequence; personal style preferences and speculative future requirements are insufficient. For related PRs, verify the repository, revision and dependency state, use full URLs, and distinguish a current defect from an unmerged dependency. + +## Browser Eval + +8. Does the change preserve the boundary between benchmark-owned task definitions/scoring and harness-owned provider execution/reporting? Are CLI entry points thin and all supported callers using the same shared behavior, without provider-specific acceptance rules or help that changes what the provider is being evaluated on? +9. Do results, denominators, timing populations and report claims follow the selected track's current profile and declared evidence? Are physical retries and corrections preserved, auxiliary artifacts verified, and measured outputs, credentials and run-local material kept outside Git? Could an input or scoring change silently rewrite frozen results or mix report targets? +10. Do the affected report views agree with the underlying evidence and each other, including generated languages, filters, highlights and failure details? Do the applicable behavior and release checks exercise those public boundaries, including evidence drilldown and hosted navigation when changed? diff --git a/.opencodereview/rule.json b/.opencodereview/rule.json new file mode 100644 index 000000000..0d7516805 --- /dev/null +++ b/.opencodereview/rule.json @@ -0,0 +1,9 @@ +{ + "rules": [ + { + "path": "**", + "rule": ".opencodereview/review-rules.md", + "merge_system_rule": true + } + ] +} From c22a20be9aa090205a898675a3bb735eb49f3515 Mon Sep 17 00:00:00 2001 From: lanyue-llk <270302213+lanyue-llk@users.noreply.github.com> Date: Tue, 27 Oct 2026 17:57:45 +0800 Subject: [PATCH 2/4] docs(review): focus validation and benchmark contract checks --- .opencodereview/review-rules.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/.opencodereview/review-rules.md b/.opencodereview/review-rules.md index ada02ba76..6be54e3ac 100644 --- a/.opencodereview/review-rules.md +++ b/.opencodereview/review-rules.md @@ -9,7 +9,7 @@ Follow this review path, checking each answer against the current diff, relevant 3. What happens with invalid or missing input, partial failure, timeouts, retries, cancellation and concurrent work where relevant? Can errors be swallowed, required checks bypassed, state corrupted, resources leaked or secrets/private data exposed? 4. Does the design apply object-oriented principles to shared components: clear responsibilities, encapsulated state and invariants, and stable interfaces that let implementations vary without changing callers? Does it reuse existing objects and upstream capabilities, favor composition over inheritance, and keep shared behavior under one owner? Could scattered conditionals, exposed internals or duplicated pipelines be replaced by a cohesive object or interface? Is the abstraction justified by current behavior, with simple stateless transformations kept as functions and no unnecessary class hierarchies? 5. Would another supported dataset, provider, cohort or report use the same implementation through explicit data or configuration? Is reusable code coupled to one report's IDs, paths, participant count, measurements or prose, and would adding another instance require edits across unrelated modules? -6. Do tests exercise the intended behavior through supported interfaces, including valid and invalid scenarios and the previously correct behavior at risk? Do fixtures reflect real producer/consumer contracts? Can a focused reproduction distinguish the defect from its passing control, rather than merely confirm the current implementation? Do validation results actually cover the changed behavior? +6. Do tests exercise the intended behavior through supported interfaces, including valid and invalid scenarios and the previously correct behavior at risk? Do fixtures reflect real producer/consumer contracts? When execution is available and permitted, have the smallest relevant reproduction and passing control been run on the reviewed revision, including the installed package when affected? 7. Have all changed files and affected shared paths been examined, including other occurrences of a suspected root cause? Where the review environment supports agents, would independent parallel checks of distinct risk areas improve coverage within OCR's execution policy? Before finishing, have candidate findings been verified and duplicates combined, including earlier findings that may still reproduce on the current revision? Report actionable findings under OCR's normal reporting rules, supported by a concrete trigger, affected behavior and code evidence. Treat design or optimization concerns as findings only when they have a demonstrable consequence; personal style preferences and speculative future requirements are insufficient. For related PRs, verify the repository, revision and dependency state, use full URLs, and distinguish a current defect from an unmerged dependency. @@ -17,5 +17,5 @@ Report actionable findings under OCR's normal reporting rules, supported by a co ## Browser Eval 8. Does the change preserve the boundary between benchmark-owned task definitions/scoring and harness-owned provider execution/reporting? Are CLI entry points thin and all supported callers using the same shared behavior, without provider-specific acceptance rules or help that changes what the provider is being evaluated on? -9. Do results, denominators, timing populations and report claims follow the selected track's current profile and declared evidence? Are physical retries and corrections preserved, auxiliary artifacts verified, and measured outputs, credentials and run-local material kept outside Git? Could an input or scoring change silently rewrite frozen results or mix report targets? +9. Do scored answers come from retained native evidence, and do results, denominators, timing populations and report claims follow the selected track contract? Are retries and corrections preserved, artifacts verified, and incomplete or uncertified cohorts excluded where certification is required? Are measured outputs, credentials and run-local material kept outside Git? Could a change silently rewrite frozen results or mix report targets? 10. Do the affected report views agree with the underlying evidence and each other, including generated languages, filters, highlights and failure details? Do the applicable behavior and release checks exercise those public boundaries, including evidence drilldown and hosted navigation when changed? From 92d55cc87515414646d0762e55724a110e9eb98b Mon Sep 17 00:00:00 2001 From: lanyue-llk <270302213+lanyue-llk@users.noreply.github.com> Date: Tue, 27 Oct 2026 18:05:52 +0800 Subject: [PATCH 3/4] docs(review): organize concise review steps --- .opencodereview/review-rules.md | 34 +++++++++++++++++++-------------- 1 file changed, 20 insertions(+), 14 deletions(-) diff --git a/.opencodereview/review-rules.md b/.opencodereview/review-rules.md index 6be54e3ac..df69bfc35 100644 --- a/.opencodereview/review-rules.md +++ b/.opencodereview/review-rules.md @@ -1,21 +1,27 @@ # Repository review supplement -These questions supplement OCR's built-in review instructions. Preserve its scope, severity thresholds, output format and execution policy; where they conflict, follow the built-in instructions. Use the questions to investigate the change, not as a checklist to repeat in the review output. Apply the repository's review-relevant constraints in `AGENTS.md` and `CONTRIBUTING.md`. +These requirements supplement OCR's built-in instructions. Keep its severity thresholds, output format and execution policy. Follow the review constraints in `AGENTS.md` and `CONTRIBUTING.md`. -Follow this review path, checking each answer against the current diff, relevant callers and evidence: +## 1. Does the PR achieve its purpose? -1. What user problem and acceptance criteria does this PR address? Does the proposed approach achieve them, and do the implementation and every changed file serve that purpose? Which claims in the description or earlier comments need independent verification? -2. How does the change behave through the supported public entry points and downstream consumers? Could it break previously correct behavior or introduce a new defect through defaults, old inputs, boundary cases or interactions outside the diff? If it restores a removed behavior, does Git history explain why it was removed and whether that reason still applies? -3. What happens with invalid or missing input, partial failure, timeouts, retries, cancellation and concurrent work where relevant? Can errors be swallowed, required checks bypassed, state corrupted, resources leaked or secrets/private data exposed? -4. Does the design apply object-oriented principles to shared components: clear responsibilities, encapsulated state and invariants, and stable interfaces that let implementations vary without changing callers? Does it reuse existing objects and upstream capabilities, favor composition over inheritance, and keep shared behavior under one owner? Could scattered conditionals, exposed internals or duplicated pipelines be replaced by a cohesive object or interface? Is the abstraction justified by current behavior, with simple stateless transformations kept as functions and no unnecessary class hierarchies? -5. Would another supported dataset, provider, cohort or report use the same implementation through explicit data or configuration? Is reusable code coupled to one report's IDs, paths, participant count, measurements or prose, and would adding another instance require edits across unrelated modules? -6. Do tests exercise the intended behavior through supported interfaces, including valid and invalid scenarios and the previously correct behavior at risk? Do fixtures reflect real producer/consumer contracts? When execution is available and permitted, have the smallest relevant reproduction and passing control been run on the reviewed revision, including the installed package when affected? -7. Have all changed files and affected shared paths been examined, including other occurrences of a suspected root cause? Where the review environment supports agents, would independent parallel checks of distinct risk areas improve coverage within OCR's execution policy? Before finishing, have candidate findings been verified and duplicates combined, including earlier findings that may still reproduce on the current revision? +Read the goal and acceptance criteria, then trace the implementation through its callers. Verify the proposed solution and each changed file serve that goal. -Report actionable findings under OCR's normal reporting rules, supported by a concrete trigger, affected behavior and code evidence. Treat design or optimization concerns as findings only when they have a demonstrable consequence; personal style preferences and speculative future requirements are insufficient. For related PRs, verify the repository, revision and dependency state, use full URLs, and distinguish a current defect from an unmerged dependency. +## 2. Could it break existing behavior? -## Browser Eval +Check working scenarios, compatibility and failure paths, including retries, cancellation and concurrent work where relevant. Investigate the shared cause of defects; consult history when restoring previously removed behavior. -8. Does the change preserve the boundary between benchmark-owned task definitions/scoring and harness-owned provider execution/reporting? Are CLI entry points thin and all supported callers using the same shared behavior, without provider-specific acceptance rules or help that changes what the provider is being evaluated on? -9. Do scored answers come from retained native evidence, and do results, denominators, timing populations and report claims follow the selected track contract? Are retries and corrections preserved, artifacts verified, and incomplete or uncertified cohorts excluded where certification is required? Are measured outputs, credentials and run-local material kept outside Git? Could a change silently rewrite frozen results or mix report targets? -10. Do the affected report views agree with the underlying evidence and each other, including generated languages, filters, highlights and failure details? Do the applicable behavior and release checks exercise those public boundaries, including evidence drilldown and hosted navigation when changed? +## 3. Is the design maintainable and reusable? + +Require sound object-oriented design: clear responsibilities, encapsulated state and stable interfaces. Reuse existing components and keep shared logic under one owner. Keep simple stateless functions simple. Avoid unnecessary abstractions and report-specific branches; another supported report or provider should use data or configuration rather than scattered code changes. + +## 4. Does the evidence prove it works? + +Check behavior tests against the requirements, including valid and invalid scenarios and regression controls. When execution is available and permitted, run focused checks on the reviewed revision. Use real producer/consumer contracts and test the installed package when affected. + +## 5. Has the whole change been reviewed? + +Inspect all changed files, affected shared paths and similar occurrences. Use parallel agents for distinct risks when supported by OCR. Recheck earlier findings on the current revision, combine duplicates and report actionable defects with a concrete scenario and code evidence. Treat design concerns as findings when they have a demonstrable consequence. + +## Repository checks + +Keep provider execution and reporting in the harness; use benchmark-owned definitions and scoring. Trace scored answers to native evidence. Verify populations, timing and certification against the selected track contract. Preserve frozen evidence and report targets, keep private run data outside Git, and check report views agree with their evidence. From 8cb740ee072c109b2140c21f8d3e7c40579665b5 Mon Sep 17 00:00:00 2001 From: lanyue-llk <270302213+lanyue-llk@users.noreply.github.com> Date: Tue, 27 Oct 2026 18:12:46 +0800 Subject: [PATCH 4/4] docs(review): retain review standards in actionable steps --- .opencodereview/review-rules.md | 33 ++++++++++++++++++++++----------- 1 file changed, 22 insertions(+), 11 deletions(-) diff --git a/.opencodereview/review-rules.md b/.opencodereview/review-rules.md index df69bfc35..cfe4c778e 100644 --- a/.opencodereview/review-rules.md +++ b/.opencodereview/review-rules.md @@ -1,26 +1,37 @@ # Repository review supplement -These requirements supplement OCR's built-in instructions. Keep its severity thresholds, output format and execution policy. Follow the review constraints in `AGENTS.md` and `CONTRIBUTING.md`. +These requirements supplement OCR's built-in instructions. Keep its severity thresholds, output format and execution policy. Follow repository review constraints in `AGENTS.md` and `CONTRIBUTING.md`. -## 1. Does the PR achieve its purpose? +## 1. Does the change meet its goal and constraints? -Read the goal and acceptance criteria, then trace the implementation through its callers. Verify the proposed solution and each changed file serve that goal. +- Read the PR, linked requirements, relevant comments, architecture and code-style guidance before judging the diff. +- Trace runtime behavior through callers; check the approach and every changed file against the acceptance criteria. Finding no defects alone does not verify completion. -## 2. Could it break existing behavior? +## 2. Could it break correct behavior? -Check working scenarios, compatibility and failure paths, including retries, cancellation and concurrent work where relevant. Investigate the shared cause of defects; consult history when restoring previously removed behavior. +- Check existing callers, inputs, data and configuration, including necessary migration paths and relevant timeout, retry, cancellation and concurrency scenarios. +- Look for swallowed errors, defaults hiding required inputs and bypassed checks. When restoring old behavior or changing defaults and fallbacks, check why the previous behavior changed and whether that reason still applies. -## 3. Is the design maintainable and reusable? +## 3. Is the design sound and reusable? -Require sound object-oriented design: clear responsibilities, encapsulated state and stable interfaces. Reuse existing components and keep shared logic under one owner. Keep simple stateless functions simple. Avoid unnecessary abstractions and report-specific branches; another supported report or provider should use data or configuration rather than scattered code changes. +- Require sound object-oriented design: clear responsibilities, encapsulated state and stable interfaces. Reuse existing components; keep shared behavior under one owner. +- Check naming, file placement and dependency direction. Choose the simplest maintainable solution; avoid unnecessary state, configuration, dependencies and class hierarchies. +- Keep simple stateless functions simple. Another supported report or provider should use data or configuration, without report-specific branches or scattered code edits. -## 4. Does the evidence prove it works? +## 4. Are upstream and cross-repository dependencies handled correctly? -Check behavior tests against the requirements, including valid and invalid scenarios and regression controls. When execution is available and permitted, run focused checks on the reviewed revision. Use real producer/consumer contracts and test the installed package when affected. +- Check locked versions against official APIs and lifecycle hooks. Custom adapters need a reproducible upstream gap, a clear protocol boundary and behavior tests. +- Verify related PR revisions and merge state; use full URLs. Distinguish a working paired version, an unmerged dependency and a current defect. -## 5. Has the whole change been reviewed? +## 5. Does validation cover the actual behavior? -Inspect all changed files, affected shared paths and similar occurrences. Use parallel agents for distinct risks when supported by OCR. Recheck earlier findings on the current revision, combine duplicates and report actionable defects with a concrete scenario and code evidence. Treat design concerns as findings when they have a demonstrable consequence. +- Check requirement-based tests for accepted/rejected scenarios and regressions, using real producer/consumer contracts. Static checks alone do not establish runtime correctness. +- Inspect validation commands and results for the reviewed revision. When permitted, run focused reproductions and passing controls, including installed-package checks when affected. Identify material validation gaps. + +## 6. Has the whole change been reviewed? + +- Inspect every changed file, affected shared paths and similar defects. Use parallel agents for distinct risks when supported by OCR; finish the affected scope before reporting. +- Recheck earlier findings, verify suspected causes and combine duplicates. Report actionable issues with their trigger, consequence, code location and suggested correction under OCR's reporting rules. ## Repository checks