Skip to content

PR-10: docs/CI: README macOS manual + rollup automation (no tests) - #101

Merged
flyingrobots merged 6 commits into
mainfrom
echo/pr-10-readme-macos-ci-local
Nov 2, 2025
Merged

flyingrobots merged 6 commits into
mainfrom
echo/pr-10-readme-macos-ci-local

Conversation

@flyingrobots

@flyingrobots flyingrobots commented Nov 1, 2025 •

Copy link
Copy Markdown
Owner

Scope hygiene: this PR is strictly docs/CI/tooling.

Changes

  • README: CI tips for macOS manual workflow + local reproduction
  • Hooks: pre-commit regenerates docs/echo-total.md on any docs/**/*.md change; aborts if rollup changed (preserve index)
  • Scripts: gen-echo-total.sh includes subdirectories; locale-stable sort; static header
  • CI: echo-total-check points contributors to 'make echo-total'
  • Docs: execution-plan + decision-log entries for the above

Note

  • Commit header tests (BLAKE3 header encoding/hash) are intentionally NOT in this PR. They live in PR-09 (branch: echo/pr-09-blake3-header-tests).

Rationale

  • Keep PRs single-purpose to simplify review and avoid duplicate diffs across PR-09/PR-10.

@coderabbitai

coderabbitai Bot commented Nov 1, 2025 •

Copy link
Copy Markdown
Contributor

Summary by CodeRabbit

  • Tests

    • Added comprehensive unit tests for commit hash validation, verifying header encoding endianness, hash equivalence checks, and parent ordering sensitivity to prevent future regressions.
  • Documentation

    • Added decision log entries documenting test additions and CI workflow improvements for contributors, enabling easier local reproduction of continuous integration checks and manual macOS testing procedures.

Walkthrough

Added unit tests for compute_commit_hash verifying header byte construction, BLAKE3 hash equivalence, little-endian encoding for version and parents-length, and sensitivity to parent ordering; plus documentation entries in decision log and execution plan describing these test additions and CI tips.

Changes

Cohort / File(s) Change Summary
BLAKE3 header unit tests
crates/rmg-core/src/snapshot.rs
Added four unit tests that construct commit header bytes and assert: commit hash equals blake3(header), version and parents-length are little-endian, and reversing parent order changes the hash. No public API changes.
Documentation / tracking
docs/decision-log.md, docs/execution-plan.md, docs/echo-total.md
Added entries for PR-09 (BLAKE3 header tests) and PR-10 (README macOS / local CI tips); recorded scope as tests-only and docs-only, with dates and short descriptions.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

  • Focus review on the new tests in crates/rmg-core/src/snapshot.rs:
    • Verify test vectors (header byte layout and endianness) are correct and unambiguous.
    • Confirm tests use stable, deterministic hashing (no nondeterministic inputs).
  • Docs changes are low-risk; check for accurate PR references and dates.

Possibly related PRs

Poem

Little-endian whispers in a header's bed,
BLAKE3 hums where bytes are led.
Swap the parents, the fingerprint sighs—
Tests stand guard with unblinking eyes.
Regressions flee; pedants sleep well-fed.

Pre-merge checks and finishing touches

❌ Failed checks (2 warnings, 1 inconclusive)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 75.00% which is insufficient. The required threshold is 80.00%. You can run @coderabbitai generate docstrings to improve docstring coverage.
Title Check ⚠️ Warning The PR title explicitly states "(no tests)" as a scope qualifier, but the changeset includes unit tests in crates/rmg-core/src/snapshot.rs that add test coverage for compute_commit_hash, including tests for header encoding, endianness, and hash equivalence. This is a direct contradiction—the title claims the PR contains no tests, yet the raw summary clearly shows test additions. The title is therefore misleading about the actual content of the changeset. Revise the PR title to accurately reflect the actual scope. Either remove "(no tests)" if tests are intentionally included, or ensure the changeset matches the claim by moving the test additions to a separate PR-09 branch as the description suggests they should be. The title must align with the actual file modifications present in the changeset.
Description Check ❓ Inconclusive The PR description explicitly states "Commit header tests (BLAKE3 header encoding/hash) are intentionally NOT in this PR. They live in PR-09 (branch: echo/pr-09-blake3-header-tests)." However, the changeset includes crates/rmg-core/src/snapshot.rs with exactly those unit tests. The description accurately describes the documentation and CI changes that ARE present, but it contains a significant factual inaccuracy regarding scope—it claims to exclude tests that are demonstrably included in the changeset, making the description contradictory and unreliable about what the PR actually contains. Resolve the scope mismatch before merge. Either update the PR description to accurately reflect that commit header tests ARE included, or remove the test file changes (snapshot.rs) from this PR and ensure they exist only in PR-09. The description must truthfully represent what the changeset contains to provide meaningful review context.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch echo/pr-10-readme-macos-ci-local

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 2

📜 Review details

Configuration used: CodeRabbit UI

Review profile: ASSERTIVE

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 4cc2107 and c687cd2.

⛔ Files ignored due to path filters (1)
  • README.md is excluded by !*.md
📒 Files selected for processing (3)
  • crates/rmg-core/src/snapshot.rs (1 hunks)
  • docs/decision-log.md (1 hunks)
  • docs/execution-plan.md (1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
  • GitHub Check: Security Audit
🔇 Additional comments (4)
crates/rmg-core/src/snapshot.rs (2)

197-219: Excellent independent verification pattern.

This test correctly constructs the expected header bytes independently of the function under test, then verifies equivalence. The spot-checks for little-endian encoding (lines 217-218) provide valuable regression protection beyond the hash equality assertion.

The use of blake3::hash on the full byte buffer (vs. incremental Hasher::update in the production code) is appropriate—it confirms that the incremental hashing in compute_commit_hash produces the same result as a single-pass hash, which is exactly what you want to validate.


221-229: Critical determinism property validated.

This test confirms that parent ordering is reflected in the commit hash, which is essential for deterministic merge commit identification. Clean and focused.

docs/decision-log.md (1)

35-36: Documentation entries are clear and well-structured.

Both entries correctly follow the established table format, provide appropriate context, and accurately scope their respective changes (tests-only for PR-09, docs-only for PR-10). The consequences are appropriately noted.

docs/execution-plan.md (1)

97-107: Execution plan entries accurately document the changes.

Both PR-09 and PR-10 entries are well-structured and follow the established format. The PR-09 description precisely matches the test code added to snapshot.rs (header bytes construction, hash equivalence, LE encoding verification). The PR-10 description mentions README changes for CI tips that are not included in the files under review.

Note: The README.md changes described in the PR-10 section (lines 104-106) are not present in the provided review files. This reinforces the inconsistency flagged earlier—if this PR is supposed to include README updates, those files should be part of the review.

Comment thread crates/rmg-core/src/snapshot.rs Outdated
Comment thread crates/rmg-core/src/snapshot.rs Outdated
coderabbitai[bot]
coderabbitai Bot previously approved these changes Nov 1, 2025
…arkdown in echo-total

- pre-commit: trigger rollup when docs/**.md changes (exclude rollup); abort commit if rollup changed to preserve index
- generator: include subdirectories (e.g., docs/guide), deterministic LC_ALL=C sort; stable header (no timestamp/SHA)
- docs: log automation in execution plan + decision log
- Keep PR-10 strictly docs/CI/tooling; tests remain in PR-09 (echo/pr-09-blake3-header-tests)
- Log decision in execution-plan + decision-log (no runtime change)
- Regenerate docs/echo-total.md
@flyingrobots flyingrobots changed the title PR-10: docs: README macOS manual workflow + local CI tips PR-10: docs/CI: README macOS manual + rollup automation (no tests) Nov 1, 2025
@flyingrobots
flyingrobots merged commit 48a3381 into main Nov 2, 2025
11 of 12 checks passed
@flyingrobots
flyingrobots deleted the echo/pr-10-readme-macos-ci-local branch November 2, 2025 00:13
@coderabbitai coderabbitai Bot mentioned this pull request Jun 1, 2026
28 tasks
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