Skip to content

skills: check artifacts and require extra self-review for large PRs - #105

Merged
hsliuustc0106 merged 2 commits into
mainfrom
codex-review-artifact-hygiene
Oct 6, 2026
Merged

hsliuustc0106 merged 2 commits into
mainfrom
codex-review-artifact-hygiene

Conversation

@hsliuustc0106

@hsliuustc0106 hsliuustc0106 commented Oct 6, 2026 •

Copy link
Copy Markdown
Contributor

Purpose

Require committed-artifact checks in System1-Omni reviews and prechecks, including quick checks. Shared guidance distinguishes maintained configuration, examples and replay/oracle fixtures from redundant run summaries, response dumps, cache/profiler output and process notes. Preserve raw measurements and failures in durable evidence or an immutable commit archive, and verify consumers, documentation links and replay commands before removing output.

Add contributor requirements for PRs with more than 3,000 changed lines of authored code: complete a full self-review, consider splitting independent changes, explain why a large change stays together with a component map and review order, and report validation for each affected component and interface. Count additions plus deletions in source, tests and build/validation scripts against the PR merge base, separately from total diff size. Documentation, generated output, lockfiles and static fixtures are excluded from this code count but still reviewed. Keep the PR in draft until contributor self-review is complete.

CONTRIBUTING.md defines the large-change policy; the canonical precheck-pr, self-review and system1-omni-review skills apply it. Size triggers closer preparation and review, without implying a correctness defect or requiring expensive experiments by itself.

Test Plan

System1-Omni Version / Commit: base 47eff9cdeda01e4847a4fb9634a43f2cab6a233f, head ef9279638e54f2b3c82403b9fd3ba3a949cdfb99.

Validate all three skill entrypoints with quick_validate.py, resolve their shared artifact and large-change links/anchors, inspect the complete instruction diff, run git diff --check, and build the documentation using the existing pinned docs environment. Check the size convention against #96. This PR changes contributor guidance and review instructions; runtime tests and GPU experiments are inapplicable.

Test Result

  • quick_validate.py passed for self-review, precheck-pr and system1-omni-review.
  • All five shared artifact/large-change references resolve; the generated contributor-guide page includes the large-code-changes anchor.
  • git diff --check and python -m mkdocs build --strict passed.
  • At jev_vl: add native inference and prefix caching #96 head 4927d2a3373ae3e2d825bf63a320178988540031, the Rust/CUDA/header sources and tests plus Python preparation/replay scripts account for 4,649 changed authored-code lines, out of 7,413 total changed text lines and one binary file. It exceeds the threshold after excluding fixtures, documentation, licenses and configuration.
  • The earlier artifact cleanup on jev_vl: add native inference and prefix caching #96 preserved all 61 replay fixture/example files byte-for-byte, restored the original manifest hash, passed all 14 benchmark/replay tests, and built its documentation strictly.

Demo / evidence

N/A for inference or performance measurements: this PR changes instructions and documentation. The concrete artifact cleanup is in #96, removing 30 generated run files and linking their immutable archive while preserving replay inputs and expected responses.

Self-review

  • I have reviewed the full diff and addressed the issues I found.
  • I have checked that the change follows the project's architecture and stays focused on the stated purpose.
  • I have run the checks appropriate to this change and reported commands, results, and anything I could not verify above.
  • I have checked that the PR description, documentation, and any accuracy or performance claims match the implementation and available evidence.

Signed-off-by: Hongsheng Liu <liuhongsheng4@huawei.com>
Signed-off-by: Hongsheng Liu <liuhongsheng4@huawei.com>
@hsliuustc0106 hsliuustc0106 changed the title skills: check committed artifacts in reviews and prechecks skills: check artifacts and require extra self-review for large PRs Oct 6, 2026
@hsliuustc0106
hsliuustc0106 marked this pull request as ready for review October 6, 2026 14:44
@hsliuustc0106
hsliuustc0106 merged commit 4a67957 into main Oct 6, 2026
4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant