Skip to content

fix(review): paginate getLatestDeploymentStatus deployments and statuses (#7805) - #7822

Closed
kai392 wants to merge 2 commits into
JSONbored:mainfrom
kai392:fix/preview-deployment-pagination-7805
Closed

fix(review): paginate getLatestDeploymentStatus deployments and statuses (#7805)#7822
kai392 wants to merge 2 commits into
JSONbored:mainfrom
kai392:fix/preview-deployment-pagination-7805

Conversation

@kai392

@kai392 kai392 commented Jul 21, 2026

Copy link
Copy Markdown
Contributor

Summary

  • Paginate getLatestDeploymentStatus deployments-list and per-deployment-statuses reads via findAcrossPages, matching the sibling functions already fixed in fix(review): paginate preview-url.ts's PR-comment and check-run GitHub reads #7469.
  • Extend findAcrossPages to accept async probes so deployment pages can nest status pagination.
  • Add regression tests for preview URLs on deployments page 2 and deployment-statuses page 2.

Closes #7805

Test plan

  • Added unit tests in test/unit/preview-url.test.ts for both pagination gaps
  • CI validate-tests + codecov/patch on src/review/**

@kai392
kai392 requested a review from JSONbored as a code owner July 21, 2026 13:38
@superagent-security

Copy link
Copy Markdown
Contributor

Superagent didn't find any vulnerabilities or security issues in this PR.

…ses (JSONbored#7805)

Reuse findAcrossPages for both GitHub list reads so preview URL discovery
does not silently miss deployments or statuses beyond page 1.

Closes JSONbored#7805

Co-authored-by: Cursor <cursoragent@cursor.com>
@codecov

codecov Bot commented Jul 21, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 25.00000% with 24 lines in your changes missing coverage. Please review.
✅ Project coverage is 88.53%. Comparing base (ead55ca) to head (ff2bea3).
⚠️ Report is 2 commits behind head on main.

Files with missing lines Patch % Lines
src/review/visual/preview-url.ts 25.00% 24 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #7822      +/-   ##
==========================================
- Coverage   88.65%   88.53%   -0.13%     
==========================================
  Files         730       89     -641     
  Lines       76959    21360   -55599     
  Branches    22787     3727   -19060     
==========================================
- Hits        68229    18911   -49318     
+ Misses       7684     2247    -5437     
+ Partials     1046      202     -844     
Flag Coverage Δ
shard-1 42.46% <25.00%> (-11.16%) ⬇️
shard-2 2.05% <0.00%> (-49.48%) ⬇️
shard-3 ?

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines Coverage Δ
src/review/visual/preview-url.ts 42.46% <25.00%> (-30.06%) ⬇️

... and 641 files with indirect coverage changes

@loopover-orb loopover-orb Bot added the gittensor:bug Gittensor-scored bug fix — scores a 0.05x multiplier. label Jul 21, 2026
@loopover-orb

loopover-orb Bot commented Jul 21, 2026

Copy link
Copy Markdown
Contributor

Caution

🛑 LoopOver review result - fixes required

Review updated: 2026-07-21 13:57:56 UTC

2 files · 1 AI reviewer · no blockers · CI failing · blocked

🛑 Suggested Action - Fix Blockers

Review summary
This PR extends findAcrossPages to accept async probes and rewires getLatestDeploymentStatus to paginate both the deployments list and each deployment's statuses, fixing the same truncation gap already patched in sibling functions (#7469). The pagination and early-exit logic look correct: page-1 URL stays bare, page cursors append correctly, and latestState/sawFailure/sawPending bookkeeping is preserved from the original loop. The two new tests exercise real page-2 scenarios (deployments page 2, statuses page 2) rather than fabricated states, matching the existing test style in this file.

Nits — 5 non-blocking
  • codecov/patch reports only 25% of the diff hit by tests — the error-path branches inside inspectDeploymentStatuses's .catch and the sawFailure/sawPending bookkeeping in the deployments probe (src/review/visual/preview-url.ts:159-176) aren't directly exercised by the two new tests, so consider adding a case for a statuses-fetch failure and a deployment with only failure/pending states and no URL.
  • The FAILED validate/validate-code/validate-tests(3) checks are undetailed and this branch is 3 commits behind default — likely explained by drift rather than this diff, but worth rebasing to confirm.
  • nit: the nested `for` loop inside the async probe callback in getLatestDeploymentStatus (src/review/visual/preview-url.ts:159) is depth-5 control flow; consider extracting the per-deployment loop body into a named helper for readability, though this is stylistic only.
  • Add a test where inspectDeploymentStatuses's findAcrossPages call throws (e.g. a statuses fetch failure) to exercise the .catch arm at src/review/visual/preview-url.ts:~172.
  • Add a test where no deployment yields a URL but latestState is 'failure' across multiple deployments, to cover the sawFailure/sawPending accumulation branches.

CI checks failing

  • validate
  • codecov/patch — 25.00% of diff hit (target 99.00%)
  • validate-tests (3)

Decision drivers

  • ✅ Code review — No blockers (1 reviewer)
  • ✅ Gate result — Passing (No configured blocker found.)
Context & advisory signals — never blocks the verdict
Signal Result Evidence
Linked issue ✅ Linked #7805
Related work ✅ No active overlap found No same-issue or scoped active PR overlap found.
Change scope ✅ 20/20 Low review scope from cached public metadata (1 linked issue).
Validation posture ✅ 25/25 PR body includes validation/test evidence.
Contributor workload ✅ 10/10 Author activity: 31 registered-repo PR(s), 21 merged, 0 issue(s).
Contributor context ✅ Confirmed Gittensor contributor kai392; Gittensor profile; 31 PR(s), 0 issue(s).
Improvement ✅ Minor risk: clean · value: minor · LLM: moderate
Linked issue satisfaction

Addressed
The diff paginates both the deployments-list and per-deployment-statuses reads via findAcrossPages (extended to support async probes), preserving sawFailure/sawPending bookkeeping and the DeploymentLookup return contract, exactly matching the sibling functions' pattern.

Review context
  • Author: kai392
  • Role context: outside_contributor
  • Public audience mode: oss maintainer
  • Lane context: Repository is configured for direct PR review.
  • Public profile languages: not available
  • Official Gittensor activity: 31 PR(s), 0 issue(s).
  • PR-specific overlap: none found.
Contributor next steps
  • Start here: Triage stale or unlinked PRs.
Signal definitions
  • Related work = same linked issue, overlapping active PRs, or title/path similarity.
  • Change scope = cached public metadata such as size labels, draft state, and review-burden hints.
  • Validation posture = whether the PR provides enough public validation/test evidence for maintainer review.
  • Contributor workload = public contributor activity and cleanup pressure, not a repo-wide quality failure.
  • Contributor context = public GitHub/Gittensor identity context; non-Gittensor status is not a blocker.
🧪 Chat with LoopOver

Ask LoopOver a question about this PR directly in a comment — grounded only in the same cached, public-safe facts shown above, never a new claim.

  • @loopover ask &lt;question&gt; answers contribution-quality Q&A with source citations and freshness.
  • @loopover chat &lt;question&gt; answers in natural prose from cached decision-pack facts via local inference (maintainer/collaborator; read-only).
  • A plain-language @loopover mention with a real question is routed to the closest matching read-only command automatically — no exact syntax required.

Full command reference: https://loopover.ai/docs/loopover-commands

🧪 Experimental — new and may change.

🟩 Safe / merged · 🟦 Advisory · 🟨 Held for review · 🟥 Blocked / closed


💰 Earn for open-source contributions like this. Gittensor lets GitHub contributors earn for the work they already do — register to start earning →.

Checked by LoopOver, a quiet PR intelligence layer for OSS maintainers.

  • Re-run LoopOver review

…e only

Avoid a redundant page-1 refetch after pagination and satisfy
exactOptionalPropertyTypes for latestState.

Co-authored-by: Cursor <cursoragent@cursor.com>
@loopover-orb

loopover-orb Bot commented Jul 21, 2026

Copy link
Copy Markdown
Contributor

LoopOver is closing this pull request on the maintainer's behalf (CI is failing (validate, codecov/patch, validate-tests (3))). This is an automated maintenance action — to pursue this change, please open a new pull request with the issues resolved. Closed PRs may be analyzed later to improve review accuracy, but they are not automatically reopened or re-reviewed.

@loopover-orb loopover-orb Bot closed this Jul 21, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

gittensor:bug Gittensor-scored bug fix — scores a 0.05x multiplier.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

getLatestDeploymentStatus doesn't paginate deployments/statuses reads, unlike its siblings in the same file

2 participants