PR-08: build/docs: make echo-total target + README note #99
Conversation
…ocs: log in plan + decision log
|
Warning Rate limit exceeded@flyingrobots has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 13 minutes and 4 seconds before requesting another review. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📒 Files selected for processing (2)
Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. Summary by CodeRabbit
WalkthroughAdds a CI workflow that enforces regeneration of docs/echo-total.md, a Makefile target to run the generator, removes the generator's timestamp header, and adds three negative tests for the motion rule plus accompanying documentation and decision-log entries. Changes
Sequence Diagram(s)sequenceDiagram
participant PR as Pull Request
participant GH as GitHub Actions
participant Repo as Repository
participant GEN as gen-echo-total.sh
participant DIFF as Diff Check
PR->>GH: open / update PR (trigger)
GH->>Repo: checkout code
GH->>GEN: run ./scripts/gen-echo-total.sh
GEN->>Repo: read docs sources
GEN->>Repo: emit docs/echo-total.md (no timestamp)
GH->>DIFF: compare generated file vs committed
alt files match
DIFF-->>GH: pass
else files differ
DIFF->>GH: produce diff
GH-->>PR: fail job (report diff)
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes
Possibly related PRs
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
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. Comment |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
flyingrobots
dismissed
coderabbitai[bot]’s stale review
The merge-base changed after approval.
…wall-clock); regenerate docs/echo-total.md
There was a problem hiding this comment.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
scripts/gen-echo-total.sh (2)
42-42: Locale-stable sort to avoid cross-machine diffs.Sorting without a fixed locale can reorder files under different environments. Make it locale-stable.
- done < <(find "$DOCS_DIR" -maxdepth 1 -type f -name "*.md" -print0 | sort -z) + done < <(find "$DOCS_DIR" -maxdepth 1 -type f -name "*.md" -print0 | LC_ALL=C sort -z)
15-16: Add Bash 4+ version guard—declare -Ais unsupported on macOS default (Bash 3.2).The
declare -Aassociative array syntax requires Bash 4.0 or newer. Your script will fatally exit on macOS dev boxes running the default Bash 3.2. Add the version guard before line 13:#!/usr/bin/env bash set -euo pipefail +if [[ -z "${BASH_VERSINFO:-}" || ${BASH_VERSINFO[0]} -lt 4 ]]; then + echo "gen-echo-total.sh requires bash>=4 (macOS: brew install bash)"; exit 2 +fi + ROOT_DIR="$(cd "$(dirname "$0")/.." && pwd)"Alternatively, refactor away from the associative array (use a flag file or grep check instead), but the guard is the minimal fix.
♻️ Duplicate comments (1)
.github/workflows/echo-total-check.yml (1)
15-25: Rollup still churns on non-doc commits; fix generator header and improve guidance.Switch gen header to “last docs commit” (see script comment) to stop false failures when only code changes. Also, point contributors to the Make target.
Apply:
- if ! git diff --quiet -- docs/echo-total.md; then - echo "docs/echo-total.md is out of date. Run scripts/gen-echo-total.sh and commit the result." >&2 + if ! git diff --quiet -- docs/echo-total.md; then + echo "docs/echo-total.md is out of date. Run 'make echo-total' (or scripts/gen-echo-total.sh) and commit the result." >&2 git --no-pager diff -- docs/echo-total.md || true exit 1 fiAfter updating the script to use a docs-scoped SHA, push a no-doc-change commit to confirm the job stays green.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
⛔ Files ignored due to path filters (1)
README.mdis excluded by!*.md
📒 Files selected for processing (7)
.github/workflows/echo-total-check.yml(1 hunks)Makefile(2 hunks)crates/rmg-core/tests/engine_motion_negative_tests.rs(1 hunks)docs/decision-log.md(1 hunks)docs/echo-total.md(3 hunks)docs/execution-plan.md(1 hunks)scripts/gen-echo-total.sh(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
crates/rmg-core/tests/engine_motion_negative_tests.rs (3)
crates/rmg-core/src/payload.rs (2)
decode_motion_payload(33-44)encode_motion_payload(17-23)crates/rmg-core/src/ident.rs (2)
make_node_id(35-40)make_type_id(27-32)crates/rmg-core/src/demo/motion.rs (1)
motion_rule(66-78)
🪛 checkmake (0.2.2)
Makefile
[warning] 6-6: Missing required phony target "all"
(minphony)
[warning] 6-6: Missing required phony target "clean"
(minphony)
[warning] 6-6: Missing required phony target "test"
(minphony)
🪛 GitHub Actions: Docs Rollup Check
docs/echo-total.md
[error] 1-1: docs/echo-total.md is out of date. Run scripts/gen-echo-total.sh and commit the result.
🪛 LanguageTool
docs/execution-plan.md
[uncategorized] ~91-~91: The official name of this software platform is spelled with a capital “H”.
Context: ...tal rollup check (CI) - Added workflow .github/workflows/echo-total-check.yml that re...
(GITHUB)
⏰ 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 (2)
docs/decision-log.md (1)
32-35: Entries align with scope; LGTM.PR-06..08 decisions are clear and match the tooling/tests added.
Makefile (1)
42-45: LGTM; pairs well with the workflow.Target is simple and discoverable. No changes needed.
…tal.md (stable header)
flyingrobots
dismissed
coderabbitai[bot]’s stale review
The merge-base changed after approval.
…enerate echo-total after merge
Adds a Makefile target to regenerate docs/echo-total.md and documents docs commands (dev build + rollup) in README. Updates execution plan and decision log per Docs Guard.