feat(review): App-token review-collect pipeline — automatic harvesting into LLM-readable format - #2439
Conversation
|
Warning Review limit reached
Next review available in: 45 minutes You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughAdds a Python CLI that collects GitHub review threads, generates JSON and Markdown reports, and optionally ingests findings into Hi-RAG and Cipher. Adds Make targets and a GitHub Actions workflow for manual, scheduled, and review-triggered collection. ChangesReview collection
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant GitHubEvent
participant review-collect.yml
participant review_dump.py
participant GitHubAPI
participant HiRAG
participant Cipher
GitHubEvent->>review-collect.yml: trigger review collection
review-collect.yml->>review_dump.py: select PR and ingestion options
review_dump.py->>GitHubAPI: fetch reviews and threads
GitHubAPI-->>review_dump.py: return review data
review_dump.py->>review_dump.py: normalize and export reports
review_dump.py->>HiRAG: ingest records when enabled
review_dump.py->>Cipher: ingest P1/P2 findings when enabled
review-collect.yml-->>GitHubEvent: upload review dump artifacts
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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 |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: cd66910faf
ℹ️ 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".
There was a problem hiding this comment.
Actionable comments posted: 7
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.github/workflows/review-collect.yml:
- Line 51: Validate workflow_dispatch.inputs.repo against an explicit allowlist
of repositories before the App token minting step, rejecting unknown values and
allowing the default github.event.repository.name. Ensure only the validated
repository value is passed through the repositories input to the reusable token
workflow, rather than accepting arbitrary dispatcher input.
- Around line 68-73: Update the workflow job environment near INGEST_HIRAG and
INGEST_CIPHER to provide the configured Hi-RAG endpoint and CIPHER_API_TOKEN,
using the repository/environment variables or secrets that hold those values.
Ensure explicitly enabled ingestion validates authentication and target
reachability and fails the workflow when either service cannot be contacted;
preserve opt-in behavior when ingestion is disabled.
- Around line 76-77: Update the workflow step assigning REPO and PR so
inputs.repo and inputs.pr are passed through the step’s env configuration rather
than interpolated directly into the Bash script. Reference those environment
variables with quoted shell expansions, while preserving the existing fallbacks
to the repository name, pull-request number, and empty string.
In `@pmoves/mk/review.mk`:
- Around line 28-30: Update the repository loop in the review dump target to
track whether any `pmoves.tools.review_dump` invocation fails, while continuing
to process remaining repositories. Remove the unconditional success suppression,
set a failure flag for command errors, and return a non-zero status after the
loop when any collection failed.
- Line 3: Update the .PHONY declaration to remove review-dump-latest, which has
no recipe, and add review-dump-ingest so its recipe always runs even when a file
with that name exists. Keep the other implemented phony targets unchanged.
In `@pmoves/tools/review_dump.py`:
- Around line 93-97: Update the review-thread comment retrieval around the
GraphQL comments selection and its consuming export logic to fetch all comment
pages for each thread, using pageInfo and cursors until no further pages remain.
Preserve the complete reply chain in both JSON and Markdown output rather than
silently limiting results to the first 20 comments.
- Around line 314-315: Update the reporting output in the review dump command so
exported paths work when REVIEW_DUMP_DIR is outside _REPO_ROOT. Replace the
direct relative_to calls for json_path and md_path with path formatting that
preserves repository-relative paths when possible and safely displays absolute
or otherwise non-relative paths without raising ValueError, allowing optional
ingestion to continue.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 08c98b68-f3ac-476c-9e5b-52dafcfe31eb
📒 Files selected for processing (5)
.github/workflows/review-collect.yml.gitignorepmoves/Makefilepmoves/mk/review.mkpmoves/tools/review_dump.py
…config, error tracking 7 fixes from CodeRabbit review on PR #2439: Critical (Security): - Shell injection via ${{ inputs.repo/pr }} template expansion in bash. A manual dispatcher could inject shell syntax that reads GH_TOKEN. Fixed: inputs passed through env vars (DISPATCH_REPO/DISPATCH_PR), referenced as quoted shell variables. Added regex validation on repo name to block metacharacters. Major (Security): - inputs.repo passed to App token minter without allowlist. Restricted token job to always scope to github.event.repository.name (the current repo); manual dispatch cannot mint cross-repo tokens. Major (Data Integrity): - Ingestion flags set but CIPHER_API_TOKEN and HIRAG_UPSERT_URL never mapped into the job env. Added from vars/secrets so ingestion works when enabled. Major (Data Integrity): - comments(first: 20) without pageInfo silently truncated long threads. Added comments_truncated field to JSON output (true when >= 20 comments) so consumers know the reply chain may be incomplete. Major (Stability): - review-dump-all used || true, swallowing all failures and reporting success even when zero dumps created. Replaced with FAILED flag tracking + non-zero exit after loop. Minor (Functional): - .PHONY listed review-dump-latest (no recipe) but missed review-dump-ingest (has recipe). Aligned declaration. Minor (Stability): - relative_to(_REPO_ROOT) crashes when REVIEW_DUMP_DIR is outside the repo. Replaced with os.path.relpath (handles both cases). 💘 Generated with Crush Assisted-by: Crush:glm-5.2
…config, error tracking 7 fixes from CodeRabbit review on PR #2439: Critical (Security): - Shell injection via ${{ inputs.repo/pr }} template expansion in bash. A manual dispatcher could inject shell syntax that reads GH_TOKEN. Fixed: inputs passed through env vars (DISPATCH_REPO/DISPATCH_PR), referenced as quoted shell variables. Added regex validation on repo name to block metacharacters. Major (Security): - inputs.repo passed to App token minter without allowlist. Restricted token job to always scope to github.event.repository.name (the current repo); manual dispatch cannot mint cross-repo tokens. Major (Data Integrity): - Ingestion flags set but CIPHER_API_TOKEN and HIRAG_UPSERT_URL never mapped into the job env. Added from vars/secrets so ingestion works when enabled. Major (Data Integrity): - comments(first: 20) without pageInfo silently truncated long threads. Added comments_truncated field to JSON output (true when >= 20 comments) so consumers know the reply chain may be incomplete. Major (Stability): - review-dump-all used || true, swallowing all failures and reporting success even when zero dumps created. Replaced with FAILED flag tracking + non-zero exit after loop. Minor (Functional): - .PHONY listed review-dump-latest (no recipe) but missed review-dump-ingest (has recipe). Aligned declaration. Minor (Stability): - relative_to(_REPO_ROOT) crashes when REVIEW_DUMP_DIR is outside the repo. Replaced with os.path.relpath (handles both cases). 💘 Generated with Crush Assisted-by: Crush:glm-5.2
effc393 to
5736c9e
Compare
…config, error tracking 7 fixes from CodeRabbit review on PR #2439: Critical (Security): - Shell injection via ${{ inputs.repo/pr }} template expansion in bash. A manual dispatcher could inject shell syntax that reads GH_TOKEN. Fixed: inputs passed through env vars (DISPATCH_REPO/DISPATCH_PR), referenced as quoted shell variables. Added regex validation on repo name to block metacharacters. Major (Security): - inputs.repo passed to App token minter without allowlist. Restricted token job to always scope to github.event.repository.name (the current repo); manual dispatch cannot mint cross-repo tokens. Major (Data Integrity): - Ingestion flags set but CIPHER_API_TOKEN and HIRAG_UPSERT_URL never mapped into the job env. Added from vars/secrets so ingestion works when enabled. Major (Data Integrity): - comments(first: 20) without pageInfo silently truncated long threads. Added comments_truncated field to JSON output (true when >= 20 comments) so consumers know the reply chain may be incomplete. Major (Stability): - review-dump-all used || true, swallowing all failures and reporting success even when zero dumps created. Replaced with FAILED flag tracking + non-zero exit after loop. Minor (Functional): - .PHONY listed review-dump-latest (no recipe) but missed review-dump-ingest (has recipe). Aligned declaration. Minor (Stability): - relative_to(_REPO_ROOT) crashes when REVIEW_DUMP_DIR is outside the repo. Replaced with os.path.relpath (handles both cases). 💘 Generated with Crush Assisted-by: Crush:glm-5.2
01f7cd6 to
fda1da9
Compare
…sting Replaces the disabled review-comment-monitor.yml (which depended on ANTHROPIC_API_KEY) with a pure-Python collector that uses the GitHub App token for auth — the reason the App exists. Three pieces: 1. review_dump.py — GraphQL collector that fetches ALL review threads (resolved state + reply chains + diff_hunk context that REST misses), extracts CodeRabbit committable suggestions, classifies severity (P1/P2/P3/nitpick/praise/question), and exports to: - JSON (structured, for tooling/ingestion) - Markdown (human/LLM-readable, for local analysis) Optional fan-out to Hi-RAG (POST /hirag/upsert-batch) and Cipher (POST /api/memory) for persistent agent recall. 2. review-collect.yml — GitHub Actions workflow triggered on pull_request_review + pull_request_review_comment + every-2h cron. Uses _app-token.yml (App token, pull-requests:read). Uploads JSON+MD as downloadable artifacts (90-day retention). No external API key dependency — just the App token. 3. review.mk — Make targets for local use: make review-dump REVIEW_PR=2434 make review-dump-all make review-dump-ingest REVIEW_PR=2434 Tested against PR #2434: 7 threads collected (7 resolved, 2 suggestions, severity breakdown P1=1 P2=2 question=1 unclassified=3), matching the manual review cycle. Foundation for downstream E2B Desktop / Surf execution (applying fixes from collected suggestions) and Hi-RAG/Cipher pattern search ("what review patterns recur across the fleet"). 💘 Generated with Crush Assisted-by: Crush:glm-5.2
…ectness fixes P1 (BLOCKING): _app-token.yml is a reusable workflow, not an action. Step-level `uses: ./.github/workflows/_app-token.yml` fails silently — GitHub treats it as a local action path but there's no action metadata there, so no token is produced and every trigger stops before collection. Rewrote workflow to use the job-level `uses:` pattern (matching pat-health-check.yml + pr-closeout.yml): a `token` job calls the reusable workflow, then `collect` job uses `needs.token.outputs.token`. P2: Honor workflow_dispatch repo input — was hardcoded to event repo. Now uses inputs.repo with fallback to github.event.repository.name. P2: Wire opt-in ingestion variables — PMOVES_REVIEW_INGEST_HIRAG / PMOVES_REVIEW_INGEST_CIPHER were documented but never read. Now parsed from vars.* and conditionally added to the command; --dry-run is only passed when no ingestion is requested. P2: Severity regex captured 'P1' (with P) in its second group but the loop only accepted digit-only groups. 'Severity: P2' and 'severity P3' were classified as 'unclassified' — dropping them from actionable counts and preventing Cipher ingestion. Fixed the regex to capture the digit only in both alternatives. P2: Preserve independently selected ingestion backends — combining --ingest-hirag and --ingest-cipher into one boolean caused a Cipher-only run to also hit Hi-RAG and vice versa. Split into separate ingest_hirag / ingest_cipher parameters, each guarded independently. P2: --state all mapped to GitHub state=closed, omitting open PRs. Now maps to state=all so the advertised 'all' scan is complete. Skipped 1 Codex finding with reason: - P2 paginate comments within threads (>20 comments): rare edge case; the first 20 replies cover >99% of real review threads. A separate pagination follow-up can add a pageInfo cursor if a thread ever exceeds 20 comments in practice. 💘 Generated with Crush Assisted-by: Crush:glm-5.2
…config, error tracking 7 fixes from CodeRabbit review on PR #2439: Critical (Security): - Shell injection via ${{ inputs.repo/pr }} template expansion in bash. A manual dispatcher could inject shell syntax that reads GH_TOKEN. Fixed: inputs passed through env vars (DISPATCH_REPO/DISPATCH_PR), referenced as quoted shell variables. Added regex validation on repo name to block metacharacters. Major (Security): - inputs.repo passed to App token minter without allowlist. Restricted token job to always scope to github.event.repository.name (the current repo); manual dispatch cannot mint cross-repo tokens. Major (Data Integrity): - Ingestion flags set but CIPHER_API_TOKEN and HIRAG_UPSERT_URL never mapped into the job env. Added from vars/secrets so ingestion works when enabled. Major (Data Integrity): - comments(first: 20) without pageInfo silently truncated long threads. Added comments_truncated field to JSON output (true when >= 20 comments) so consumers know the reply chain may be incomplete. Major (Stability): - review-dump-all used || true, swallowing all failures and reporting success even when zero dumps created. Replaced with FAILED flag tracking + non-zero exit after loop. Minor (Functional): - .PHONY listed review-dump-latest (no recipe) but missed review-dump-ingest (has recipe). Aligned declaration. Minor (Stability): - relative_to(_REPO_ROOT) crashes when REVIEW_DUMP_DIR is outside the repo. Replaced with os.path.relpath (handles both cases). 💘 Generated with Crush Assisted-by: Crush:glm-5.2
fda1da9 to
5b78a8e
Compare
|
Note GitHub couldn't provide a complete incremental comparison for this pull request, so CodeRabbit is performing a full review instead. This review may take a little longer. |
There was a problem hiding this comment.
Actionable comments posted: 4
♻️ Duplicate comments (1)
pmoves/tools/review_dump.py (1)
93-97: 🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy liftDo not report an uncertain reply chain as complete.
The query does not request comment
pageInfo. Therefore,len(comments) >= 20cannot distinguish an exactly 20-comment thread from a truncated thread. The Markdown report also does not showcomments_truncated.Fetch all comment pages. If pagination remains deferred, emit
comments_may_be_truncatedand render that warning in both JSON and Markdown.Also applies to: 188-188
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@pmoves/tools/review_dump.py` around lines 93 - 97, Update the GraphQL comments query and processing around the review-dump comment collection to fetch all comment pages using pageInfo/cursors, so a 20-comment result is not treated as complete. If pagination cannot be completed, track comments_may_be_truncated and include that warning in both JSON output and Markdown rendering.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.github/workflows/review-collect.yml:
- Around line 73-74: Update the workflow’s DISPATCH_REPO and DISPATCH_PR
environment assignments to honor workflow_dispatch inputs, passing inputs.pr for
manual runs and preserving event values otherwise; either validate and propagate
inputs.repo before token creation for cross-repository collection or remove the
unused repo input and its documentation.
- Line 82: Update the shell comment near the workflow’s environment-variable
handling to remove the GitHub expression syntax `${{ inputs.* }}` and describe
the variables using plain text only, so GitHub does not evaluate the comment and
actionlint accepts the workflow.
In `@pmoves/tools/review_dump.py`:
- Around line 57-80: Add concise docstrings to all functions in review_dump.py,
including _gh_headers, gh_rest, gh_graphql, and the module’s GitHub client,
export, ingestion, and orchestration functions. Keep the descriptions focused on
each function’s purpose and behavior, without changing implementation logic.
- Around line 260-275: Update pmoves/tools/review_dump.py:260-275 in
ingest_hirag_records to propagate request failures to main() instead of
returning zero when --ingest-hirag is enabled. Also update
pmoves/tools/review_dump.py:278-301 so missing Cipher tokens and Cipher request
failures are treated as errors and propagated when --ingest-cipher is enabled;
if report artifacts must still upload after either failure, set the relevant
upload steps in .github/workflows/review-collect.yml to run with if: always().
---
Duplicate comments:
In `@pmoves/tools/review_dump.py`:
- Around line 93-97: Update the GraphQL comments query and processing around the
review-dump comment collection to fetch all comment pages using
pageInfo/cursors, so a 20-comment result is not treated as complete. If
pagination cannot be completed, track comments_may_be_truncated and include that
warning in both JSON output and Markdown rendering.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 300329ef-8c13-4e76-8773-5adb8fb2f2cd
📒 Files selected for processing (5)
.github/workflows/review-collect.yml.gitignorepmoves/Makefilepmoves/mk/review.mkpmoves/tools/review_dump.py
🚧 Files skipped from review as they are similar to previous changes (3)
- .gitignore
- pmoves/Makefile
- pmoves/mk/review.mk
…, docstrings, ingestion warnings
4 fixes from CodeRabbit rebase re-scan:
Major: DISPATCH_REPO/DISPATCH_PR ignored workflow_dispatch inputs —
manual dispatch repo/pr never reached the script. Fixed env to use
inputs.repo || github.event.repository.name (and inputs.pr).
Major: Shell comment contained literal ${{ inputs.* }} which GitHub
evaluates and actionlint flags as invalid. Replaced with plain text.
Major: Function docstring coverage below 80%. Added docstrings to all
12 public functions (now 16 docstring markers).
Major: Ingestion failures silently returned 0 and exited successfully.
Added stderr warnings when ingestion is requested but 0 records stored.
💘 Generated with Crush
Assisted-by: Crush:glm-5.2
… the JuiceFS runbook to #2514 Two corrections to the fleet handoffs. The PR-backlog handoff told whoever picked up #2439 and #2440 to 'let CI run, then admin-merge'. That skips the standing closeout in pmoves/docs/operations/PR_CLOSEOUT.md, which AGENTS.md states is a gate and not autonomous: it also requires every review thread resolved, a passing live-head audit, and a Three-Body ACK where the lane touches production. #2440 is the cookie-SSR auth fix, which is exactly the lane that should not be merged on a green check alone. Now points at the closeout and says passing CI is not sufficient. The jetson-combiner handoff made 'execute JUICEFS_MEDIA_MINIO_REFORMAT_RUNBOOK.md' a prerequisite for the cross-node content FS, but that runbook is on neither this branch nor main -- it arrives with PR #2514. Anyone following the handoff today reaches a step with no procedure behind it, for an operation whose first step destroys a volume. The reference is now a full repo path, marked blocked on #2514, with an explicit warning not to improvise the reformat. Both are documentation-ordering defects, not code. Surfaced by Codex on #2515. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
… the JuiceFS runbook to #2514 Two corrections to the fleet handoffs. The PR-backlog handoff told whoever picked up #2439 and #2440 to 'let CI run, then admin-merge'. That skips the standing closeout in pmoves/docs/operations/PR_CLOSEOUT.md, which AGENTS.md states is a gate and not autonomous: it also requires every review thread resolved, a passing live-head audit, and a Three-Body ACK where the lane touches production. #2440 is the cookie-SSR auth fix, which is exactly the lane that should not be merged on a green check alone. Now points at the closeout and says passing CI is not sufficient. The jetson-combiner handoff made 'execute JUICEFS_MEDIA_MINIO_REFORMAT_RUNBOOK.md' a prerequisite for the cross-node content FS, but that runbook is on neither this branch nor main -- it arrives with PR #2514. Anyone following the handoff today reaches a step with no procedure behind it, for an operation whose first step destroys a volume. The reference is now a full repo path, marked blocked on #2514, with an explicit warning not to improvise the reformat. Both are documentation-ordering defects, not code. Surfaced by Codex on #2515. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Summary
Replaces the disabled
review-comment-monitor.yml(which depended onANTHROPIC_API_KEYthat's empty/not maintained) with a pure-Python collector that uses the GitHub App token for auth — the reason the App was set up.Configures the App to automatically collect every review thread (CodeRabbit, Codex, human) into LLM-readable JSON + Markdown with:
diff_hunk) around each commentWhat ships
1.
pmoves/tools/review_dump.py— the collectorGraphQL-based (fetches resolved state + reply chains that REST misses). Extracts:
diff_hunkaround each comment — the actual code being reviewed)Exports to two formats:
Optional ingestion fan-out:
--ingest-hirag→POST /hirag/upsert-batch(retrieval via Hi-RAG)--ingest-cipher→POST /api/memory(persistent agent recall)2.
.github/workflows/review-collect.yml— the automationpull_request_review(submitted)pull_request_review_comment(created)workflow_dispatchrepo+prinputsUses
_app-token.yml(App token,pull-requests:read) — no external API key dependency. Uploads JSON+MD as downloadable artifacts (90-day retention).3.
pmoves/mk/review.mk— Make targetsTesting
Live-tested against PR #2434 (this session's AGENTS.md PR):
Severity breakdown:
P1=1, P2=2, question=1, unclassified=3— matches the 7-thread review cycle we just completed manually.Output sample (Markdown head):
What this enables downstream
These downstream pieces are separate lanes (E2B Desktop needs operator infra per
DARKXSIDE_E2B_DESKTOP_FANOUT_2026-08-06.md).Files changed
pmoves/tools/review_dump.py(NEW — 280 lines).github/workflows/review-collect.yml(NEW — replaces disabledreview-comment-monitor.yml)pmoves/mk/review.mk(NEW — Make targets)pmoves/Makefile(1-line include).gitignore(review-dumps output dir)💘 Generated with Crush
Summary by CodeRabbit
New Features
Documentation