Skip to content

feat: add bounded PR detail fill command#89

Merged
steipete merged 5 commits into
openclaw:mainfrom
TurboTheTurtle:fill-pr-details-83
Jul 17, 2026
Merged

feat: add bounded PR detail fill command#89
steipete merged 5 commits into
openclaw:mainfrom
TurboTheTurtle:fill-pr-details-83

Conversation

@TurboTheTurtle

@TurboTheTurtle TurboTheTurtle commented Jul 1, 2026

Copy link
Copy Markdown
Contributor

Closes #83.

What Problem This Solves

Adds a bounded fill-pr-details workflow for hydrating missing pull-request detail rows without allowing one started batch to consume through an operator's configured GitHub rate-limit reserve.

Why This Change Was Made

The command selects missing rows in ordered batches, bootstraps the relevant core and GraphQL quotas, and reserves quota before each request dispatched by that fill invocation. Reserve-enabled runs fail closed on redirects and stop cleanly at the threshold; ordinary sync behavior is unchanged.

The guarantee covers requests issued by this command invocation. Other tools or invocations sharing the same token remain outside its accounting.

User Impact

Operators can backfill missing PR details with bounded selection, progress output, and a strict per-invocation quota reserve. If a batch reaches the reserve, the command stops before the next request and does not persist partial PR-detail state.

Evidence

A reserve-crossing regression starts at reserve + 1, permits one repository request, blocks the next request at the configured reserve, reports rate-limit-reserve, and confirms no PR-detail row was partially persisted.

Live Boundary Proof

This ran from published head 0e54d93 against GitHub.com using an isolated archive containing PR #89 with no detail rows. An immediately preceding authenticated GitHub response header selected the reserve, with no intervening API calls. /rate_limit is zero-cost; the repository GET is the single permitted core request.

live_core_response_remaining_before=4953
reserve=4952
[github] request GET /rate_limit
[github] request GET /repos/openclaw/gitcrawl
{
  "selected": 1,
  "filled": 0,
  "remaining": 1,
  "stopped_reason": "rate-limit-reserve",
  "rate_limit": {
    "resource": "core",
    "remaining": 4952,
    "low": true
  },
  "reserve_rate_limit": 4952
}

Post-run archive state confirms no partial PR-detail persistence:

{
  "pull_requests_with_details": 0,
  "missing_pr_details": 1,
  "pr_files": 0,
  "pr_commits": 0,
  "pr_checks": 0,
  "workflow_runs": 0
}

@clawsweeper

clawsweeper Bot commented Jul 1, 2026

Copy link
Copy Markdown

Codex review: needs maintainer review before merge. Reviewed July 15, 2026, 6:57 PM ET / 22:57 UTC.

Summary
Adds a fill-pr-details CLI command that selects missing PR-detail rows in bounded batches, reports progress, and enforces a per-invocation GitHub rate-limit reserve before each request.

Reproducibility: not applicable. as a bug reproduction. The PR instead provides high-confidence live GitHub output and focused regression coverage demonstrating the proposed reserve boundary and persistence behavior.

Review metrics: 3 noteworthy metrics.

  • Changed surface: 9 files; 1,507 additions and 15 deletions. This is a substantial new CLI and GitHub-client contract despite its focused operator use case.
  • New command surface: 1 command with 7 documented operational flags. Maintainers should explicitly accept the long-term CLI and JSON compatibility burden.
  • Validation surface: 916 test additions across 4 test files. The patch includes extensive focused coverage for selection, quota boundaries, concurrency, progress, and persistence behavior.

Root-cause cluster
Relationship: fixed_by_candidate
Canonical: #83
Summary: This PR is the direct implementation candidate for the canonical bounded PR-detail hydration request; the parent archive-friction and coverage reports are related but distinct.

Members:

Proposal only: this assessment does not dispatch repair, suppress jobs, mutate sibling items, close, or merge anything.

Merge readiness
Overall: 🦞 diamond lobster
Proof: 🦞 diamond lobster
Patch quality: 🦞 diamond lobster
Result: ready for maintainer review.

Overall follows the weaker of proof and patch quality, so missing proof can cap an otherwise strong patch.

Risk before merge

  • [P1] This establishes a new public CLI and strict request-budget contract whose long-term support, naming, and semantics need explicit maintainer acceptance.
  • [P1] The guarantee is deliberately limited to requests issued through this command's client; another process or tool sharing the token can consume quota concurrently, so the reserve is not a global token-level guarantee.

Maintainer options:

  1. Approve the quota contract (recommended)
    A maintainer can explicitly accept the per-invocation guarantee and the documented exclusion of other token consumers before merging.
  2. Narrow before merge
    Reduce the initial public command or flag surface if maintainers do not want to support the full workflow contract yet.
  3. Pause the feature
    Close or defer the PR if targeted sync orchestration remains the preferred product boundary.

Next step before merge

  • [P2] A maintainer should decide whether to adopt the new public command and invocation-scoped quota contract; there is no remaining mechanical defect for an automated repair lane.

Maintainer decision needed

  • Question: Should Gitcrawl adopt fill-pr-details and its strict per-invocation rate-limit reserve as supported public CLI and client behavior?
  • Rationale: The patch is technically well-supported, but it adds a substantial command, flags, JSON contract, and quota policy that current repository behavior does not already define.
  • Likely owner: steipete — Their merged history most directly connects the PR-detail, CLI, and rate-limit surfaces whose permanent product contract must be chosen.
  • Options:
    • Adopt the bounded command (recommended): Accept the dedicated CLI and strict request-level reserve contract as supported core behavior after ordinary maintainer review.
    • Narrow the public surface: Keep the underlying missing-detail and reserve primitives but request a smaller command or fewer initially supported flags.
    • Decline the new workflow: Continue using targeted sync --numbers --with pr-details orchestration rather than adding another public command.

Security
Cleared: The diff adds no dependency, workflow, permission, secret-access, artifact-download, package-resolution, or supply-chain change; the new network behavior remains within the existing GitHub client.

Review details

Best possible solution:

Adopt the dedicated command only if maintainers want this as supported core surface, retaining strict per-request enforcement, explicit per-invocation scope, safe resumability, and unchanged default sync behavior.

Do we have a high-confidence way to reproduce the issue?

Not applicable as a bug reproduction. The PR instead provides high-confidence live GitHub output and focused regression coverage demonstrating the proposed reserve boundary and persistence behavior.

Is this the best way to solve the issue?

Yes technically: request-level reservation inside a dedicated bounded command is narrower and more reliable than the earlier batch-level enforcement. Product acceptance remains a maintainer decision because this introduces new permanent CLI and quota semantics.

AGENTS.md: unclear because the file could not be read completely.

Codex review notes: model internal, reasoning high; reviewed against b38e3152b871.

Label changes

Label changes:

  • add proof: sufficient: Contributor real behavior proof is sufficient. The PR body includes after-fix live GitHub output showing the exact reserve transition and post-run database state with no partial details; any further evidence should continue redacting tokens, endpoints, and other private data.
  • add rating: 🦞 diamond lobster: Overall readiness is 🦞 diamond lobster; proof is 🦞 diamond lobster and patch quality is 🦞 diamond lobster.
  • add status: 👀 ready for maintainer look: ClawSweeper has no concrete contributor-facing blocker left for this PR. Sufficient (live_output): The PR body includes after-fix live GitHub output showing the exact reserve transition and post-run database state with no partial details; any further evidence should continue redacting tokens, endpoints, and other private data.
  • remove status: 📣 needs proof: Current PR status label is status: 👀 ready for maintainer look.
  • remove rating: 🦪 silver shellfish: Current PR rating is rating: 🦞 diamond lobster, so this older rating label is no longer current.

Label justifications:

  • P3: This is an additive maintainer-archive workflow improvement rather than a regression or urgent user-facing failure.
  • merge-risk: 🚨 other: Merging creates a strict but invocation-scoped quota guarantee whose operator expectations and permanent product contract are not settled by green CI.
  • rating: 🦞 diamond lobster: Overall readiness is 🦞 diamond lobster; proof is 🦞 diamond lobster and patch quality is 🦞 diamond lobster.
  • status: 👀 ready for maintainer look: ClawSweeper has no concrete contributor-facing blocker left for this PR. Sufficient (live_output): The PR body includes after-fix live GitHub output showing the exact reserve transition and post-run database state with no partial details; any further evidence should continue redacting tokens, endpoints, and other private data.
  • proof: sufficient: Contributor real behavior proof is sufficient. The PR body includes after-fix live GitHub output showing the exact reserve transition and post-run database state with no partial details; any further evidence should continue redacting tokens, endpoints, and other private data.
Evidence reviewed

What I checked:

  • Prior blocker addressed: The latest commit adds client-level request serialization and reserve tracking so quota is checked and reserved at the individual request boundary rather than only before a batch. (internal/github/client.go:55, 0e54d935b5d1)
  • Live boundary proof: The PR body records a live GitHub.com run starting at reserve plus one: one repository request was dispatched, the next was stopped with rate-limit-reserve, and the archive retained zero PR-detail rows and related child rows. (internal/cli/app.go:2847, 0e54d935b5d1)
  • Focused regression coverage: The current patch adds reserve-crossing, stale-snapshot, concurrency, redirect, retry, progress, selection, and no-partial-persistence tests across the CLI and GitHub client. (internal/github/client_test.go:522, 0e54d935b5d1)
  • Existing behavior remains opt-in: The reserve is supplied through new client options and the dedicated command; ordinary sync operation is documented as unchanged, while release notifications are suppressed for reserve-enabled fills to avoid an unaccounted startup request. (internal/cli/releasecheck.go:39, 0e54d935b5d1)
  • Canonical request: This PR directly implements the still-open bounded PR-detail hydration request, including missing-row selection, ordering, limits, progress, quota reporting, safe stopping, and resumability. (internal/store/pull_request_fill.go:9, 0e54d935b5d1)

Likely related people:

  • steipete: The PR-detail hydration, central CLI dispatch, and shared GitHub rate-limit behavior appear to originate primarily from this person's merged history, making them the strongest product and compatibility routing candidate. (role: feature owner; confidence: high; commits: 7222fef197b4, 7598e3bcc54f, 300090ef7a62; files: internal/cli/app.go, internal/cli/gh_rate_limit.go, internal/github/client.go)
  • vincentkoc: Recent repository history connects this person to substantial work across the central CLI and GitHub-client surfaces affected by the new command. (role: recent area contributor; confidence: medium; files: internal/cli/app.go, internal/cli/gh_rate_limit.go, internal/github/client.go)
What the crustacean ranks mean
  • 🦀 challenger crab: rare, exceptional readiness with strong proof, clean implementation, and convincing validation.
  • 🦞 diamond lobster: very strong readiness with only minor maintainer review expected.
  • 🐚 platinum hermit: good normal PR, likely mergeable with ordinary maintainer review.
  • 🦐 gold shrimp: useful signal, but proof or patch confidence is still limited.
  • 🦪 silver shellfish: thin signal; proof, validation, or implementation needs work.
  • 🧂 unranked krab: not merge-ready because proof is missing/unusable or there are serious correctness or safety concerns.
  • 🌊 off-meta tidepool: rating does not apply to this item.

Shiny media proof means a screenshot, video, or linked artifact directly shows the changed behavior. Runtime, network, CSP, and security claims still need visible diagnostics.

How this review workflow works
  • ClawSweeper keeps one durable marker-backed review comment per issue or PR.
  • Re-runs edit this comment so the latest verdict, findings, and automation markers stay together instead of adding duplicate bot comments.
  • A fresh review can be triggered by eligible @clawsweeper re-review comments, exact-item GitHub events, scheduled/background review runs, or manual workflow dispatch.
  • PR/issue authors and users with repository write access can comment @clawsweeper re-review or @clawsweeper re-run on an open PR or issue to request a fresh review only.
  • Maintainers can also comment @clawsweeper review to request a fresh review only.
  • Fresh-review commands do not start repair, autofix, rebase, CI repair, or automerge.
  • Maintainer-only repair and merge flows require explicit commands such as @clawsweeper autofix, @clawsweeper automerge, @clawsweeper fix ci, or @clawsweeper address review.
  • Maintainers can comment @clawsweeper explain to ask for more context, or @clawsweeper stop to stop active automation.
Review history (4 earlier review cycles)
  • reviewed 2026-07-01T20:10:59.960Z sha e0621ab :: needs maintainer review before merge. :: none
  • reviewed 2026-07-09T19:19:35.428Z sha e0621ab :: needs maintainer review before merge. :: none
  • reviewed 2026-07-10T18:13:28.536Z sha b1ee4a2 :: found issues before merge. :: [P1] Preserve the requested rate-limit reserve within each batch
  • reviewed 2026-07-15T22:16:29.651Z sha 0e54d93 :: needs real behavior proof before merge. :: none

@clawsweeper clawsweeper Bot added rating: 🧂 unranked krab Not merge-ready due to missing proof or serious correctness/safety concerns. status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask. P3 Low-risk cleanup, docs, polish, ergonomics, or speculative feature. labels Jul 1, 2026
@TurboTheTurtle

Copy link
Copy Markdown
Contributor Author

@clawsweeper re-review

@clawsweeper

clawsweeper Bot commented Jul 1, 2026

Copy link
Copy Markdown

🦞🧹
ClawSweeper re-review requested.

I asked ClawSweeper to review this item again.
Action: item re-review queued (workflow sweep.yml, event repository_dispatch).
Result: the existing ClawSweeper review comment will be edited in place when the review finishes.

@clawsweeper clawsweeper Bot added proof: sufficient Contributor real behavior proof is sufficient. rating: 🦪 silver shellfish Thin PR readiness signal; proof, validation, or implementation needs work. status: ⏳ waiting on author ClawSweeper has contributor-facing work open and is waiting for author action. rating: 🦐 gold shrimp Decent PR readiness signal, but merge confidence is limited. merge-risk: 🚨 other 🚨 Merging this PR has meaningful risk outside the owned taxonomy. and removed rating: 🧂 unranked krab Not merge-ready due to missing proof or serious correctness/safety concerns. status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask. rating: 🦪 silver shellfish Thin PR readiness signal; proof, validation, or implementation needs work. labels Jul 1, 2026
@TurboTheTurtle

Copy link
Copy Markdown
Contributor Author

@clawsweeper re-review

@clawsweeper

clawsweeper Bot commented Jul 1, 2026

Copy link
Copy Markdown

🦞🧹
ClawSweeper re-review requested.

I asked ClawSweeper to review this item again.
Action: item re-review queued (workflow sweep.yml, event repository_dispatch).
Result: the existing ClawSweeper review comment will be edited in place when the review finishes.

@clawsweeper clawsweeper Bot added rating: 🐚 platinum hermit Good normal PR readiness with ordinary maintainer review expected. status: 👀 ready for maintainer look ClawSweeper has no concrete contributor-facing blocker left for this PR. and removed rating: 🦐 gold shrimp Decent PR readiness signal, but merge confidence is limited. status: ⏳ waiting on author ClawSweeper has contributor-facing work open and is waiting for author action. labels Jul 1, 2026
@clawsweeper clawsweeper Bot mentioned this pull request Jul 2, 2026
@clawsweeper clawsweeper Bot added rating: 🦐 gold shrimp Decent PR readiness signal, but merge confidence is limited. status: ⏳ waiting on author ClawSweeper has contributor-facing work open and is waiting for author action. and removed rating: 🐚 platinum hermit Good normal PR readiness with ordinary maintainer review expected. status: 👀 ready for maintainer look ClawSweeper has no concrete contributor-facing blocker left for this PR. labels Jul 10, 2026
@TurboTheTurtle TurboTheTurtle changed the title Add bounded PR detail fill command feat: add bounded PR detail fill command Jul 15, 2026
@TurboTheTurtle

Copy link
Copy Markdown
Contributor Author

@clawsweeper re-review

@clawsweeper

clawsweeper Bot commented Jul 15, 2026

Copy link
Copy Markdown

🦞🧹
ClawSweeper re-review requested.

I asked ClawSweeper to review this item again.
Action: item re-review queued (workflow sweep.yml, event repository_dispatch).
Result: the existing ClawSweeper review comment will be edited in place when the review finishes.

Re-review progress:

@clawsweeper clawsweeper Bot added rating: 🦪 silver shellfish Thin PR readiness signal; proof, validation, or implementation needs work. status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask. proof: sufficient Contributor real behavior proof is sufficient. rating: 🦞 diamond lobster Very strong PR readiness with only minor maintainer review expected. status: 👀 ready for maintainer look ClawSweeper has no concrete contributor-facing blocker left for this PR. and removed proof: sufficient Contributor real behavior proof is sufficient. rating: 🦐 gold shrimp Decent PR readiness signal, but merge confidence is limited. status: ⏳ waiting on author ClawSweeper has contributor-facing work open and is waiting for author action. rating: 🦪 silver shellfish Thin PR readiness signal; proof, validation, or implementation needs work. status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask. labels Jul 16, 2026
steipete and others added 2 commits July 16, 2026 20:21
Co-authored-by: Andy Ye <35905412+TurboTheTurtle@users.noreply.github.com>
@steipete

Copy link
Copy Markdown
Contributor

Maintainer repair proof for a280349411d7e9b886dd424050bb0696badcc5c8:

  • GOWORK=off make test-coverage — passed, total coverage 85.1%.
  • GOWORK=off go vet ./... — passed.
  • govulncheck ./... — no vulnerabilities found.
  • deadcode, module-tidiness, formatting, real build/smoke, and ./scripts/test-release.sh — passed.
  • Built-binary simulation: a mock shared token returned core quota remaining=1500; fill-pr-details selected 1, filled 0, left 1, stopped with rate-limit-reserve, and reported the default floor 1500. The mock request ledger contained only GET /rate_limit, proving work stopped before a repository/detail request.
  • Separate-client regression: two independent clients shared one mock quota; after the first dispatched, the second re-probed /rate_limit, observed 10 remaining, and stopped at the configured floor.
  • AutoReview: one semantics finding accepted; help/docs now state the cross-process race is best-effort and explain the default 1500 concurrency headroom. Follow-up review clean with no actionable findings.
  • Exact-head hosted checks: macOS, Ubuntu, Docker, and secret scan all successful; merge state CLEAN.

@steipete
steipete merged commit ffae350 into openclaw:main Jul 17, 2026
4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

merge-risk: 🚨 other 🚨 Merging this PR has meaningful risk outside the owned taxonomy. P3 Low-risk cleanup, docs, polish, ergonomics, or speculative feature. proof: sufficient Contributor real behavior proof is sufficient. rating: 🦞 diamond lobster Very strong PR readiness with only minor maintainer review expected. status: 👀 ready for maintainer look ClawSweeper has no concrete contributor-facing blocker left for this PR.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Add bounded PR-detail fill

2 participants