Skip to content

fix(node): bound federated repository aggregation - #409

Open
euxaristia wants to merge 2 commits into
Gitlawb:mainfrom
euxaristia:codex/fix-federated-response-bounds
Open

fix(node): bound federated repository aggregation#409
euxaristia wants to merge 2 commits into
Gitlawb:mainfrom
euxaristia:codex/fix-federated-response-bounds

Conversation

@euxaristia

@euxaristia euxaristia commented Sep 7, 2026

Copy link
Copy Markdown
Contributor

Summary

  • cap each peer repository page by response bytes and decoded rows
  • limit peer fan-out concurrency and keep the timeout around the complete response read
  • cap aggregate rows and serialized response bytes, report truncation, and cancel outstanding work at the ceiling

No direct issue matching this federated response-boundary fix was found in the GitHub issue or pull request index.

Changes

  • request the existing 200-row peer page instead of the legacy unpaged route
  • accept at most 512 KiB and 200 repository objects from one peer
  • run at most four peer fetches concurrently
  • return at most 1,000 repositories and 2 MiB of serialized repository data
  • add a truncated response field and count peers that returned a valid page

Test plan

  • cargo fmt --all -- --check
  • cargo check --workspace --all-targets
  • cargo clippy -p gitlawb-node --bin gitlawb-node -- -D warnings
  • cargo clippy -p gitlawb-node --all-targets -- -D warnings -A dead-code
  • exact unit tests for per-peer byte/row ceilings, aggregate row/byte ceilings, and bounded concurrency with cancellation

Summary by CodeRabbit

  • New Features

    • Federated repository results now report when a peer page is truncated.
    • Aggregated results preserve truncation status when any peer response is capped.
  • Bug Fixes

    • Valid peer-reported totals now correctly identify pages with additional rows.
    • Malformed optional total-count values no longer incorrectly mark results as truncated.
    • Aggregated results no longer report complete results when a peer page was capped.

@github-actions github-actions Bot added the needs-issue PR has no linked issue label Sep 7, 2026
@github-actions

github-actions Bot commented Sep 7, 2026

Copy link
Copy Markdown
Contributor

Thanks for the contribution. A couple of things will help us review this faster:

  • Link the issue this addresses (Closes #123). For protocol changes, open an issue first.

See CONTRIBUTING.md. Update the PR and these notes will clear automatically.

@coderabbitai

coderabbitai Bot commented Sep 7, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Team

Run ID: 8627c165-dd27-49b2-99e1-dd9641d7a1f9

📥 Commits

Reviewing files that changed from the base of the PR and between 31a14c7 and 91dd5d1.

📒 Files selected for processing (1)
  • crates/gitlawb-node/src/api/repos.rs

Limit details: You’ve used the included review currently available.


📝 Walkthrough

Walkthrough

Federated peer pages now read X-Total-Count and mark capped pages as truncated when peers report additional rows. Aggregated results preserve this state. Malformed optional totals remain non-truncating. Tests cover detection and propagation.

Changes

Federated truncation propagation

Layer / File(s) Summary
Peer truncation state
crates/gitlawb-node/src/api/repos.rs
Peer response models store page and aggregate truncation state. Valid X-Total-Count values mark a page truncated when they exceed the returned row count.
Aggregate truncation reporting
crates/gitlawb-node/src/api/repos.rs
Federated listing propagates peer truncation into the combined result. Tests cover valid totals, malformed totals, aggregation, and concurrency fixtures.

Estimated code review effort: 2 (Simple) | ~10 minutes

Merge Risk: ⚪ Minimal · up to 91dd5

Federated repository results now report truncation when a peer indicates additional rows beyond its returned page, preventing incomplete aggregate results from appearing complete. No current merge-blocking risk remains.

Suggested reviewers: beardthelion

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 41.18% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 17 functions across 1 files. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly identifies the main change: bounding federated repository aggregation.
Description check ✅ Passed The description explains the motivation, implementation changes, limits, truncation behavior, cancellation, and test coverage. It does not reproduce every template heading or checklist item, but it pr…
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
  • Fix all pre-merge checks with AI
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Usage-based review receipt

Note

This review was completed with usage-based billing: files reviewed beyond your plan's included limits are billed at $0.25/file. View usage-based billing.


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

@greptile-apps

greptile-apps Bot commented Sep 7, 2026

Copy link
Copy Markdown

Greptile Summary

The PR bounds federated repository aggregation to control peer response size, concurrency, timeout duration, aggregate rows, and serialized output size.

  • Requests a maximum 200-row page from each reachable peer.
  • Limits peer bodies to 512 KiB and runs at most four fetches concurrently.
  • Caps the aggregate at 1,000 repositories and 2 MiB, cancelling outstanding work at saturation.
  • Adds peer-count and truncation metadata, but does not propagate per-peer page truncation.

Confidence Score: 4/5

The incomplete peer-page reporting should be fixed before merging because clients can receive an incomplete federated repository set marked as untruncated.

The new request intentionally fetches only the first 200 rows from each peer, but it discards the peer's total-count header and derives truncated solely from aggregate saturation.

Files Needing Attention: crates/gitlawb-node/src/api/repos.rs

Important Files Changed

Filename Overview
crates/gitlawb-node/src/api/repos.rs Adds bounded peer fetching and aggregate serialization, but fails to mark the result truncated when an individual peer has more than 200 visible repositories.

Reviews (1): Last reviewed commit: "fix(node): bound federated repository ag..." | Re-trigger Greptile

Comment thread crates/gitlawb-node/src/api/repos.rs
@beardthelion beardthelion added crate:node gitlawb-node — the serving node and REST API kind:bug Defect fix — wrong or unsafe behavior labels Sep 7, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

crate:node gitlawb-node — the serving node and REST API kind:bug Defect fix — wrong or unsafe behavior needs-issue PR has no linked issue

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants