feat(events): cap the default page at one API page - #1766
Conversation
|
This pull request is part of a Mergify stack:
|
Merge Protections🟢 All 6 merge protections satisfied — ready to merge. Show 6 satisfied protections🟢 🤖 Continuous Integration
🟢 👀 Review Requirements
🟢 Enforce conventional commitMake sure that we follow https://www.conventionalcommits.org/en/v1.0.0/
🟢 🔎 Reviews
🟢 📕 PR description
🟢 🚦 Auto-queueWhen all merge protections are satisfied, this pull request will be queued automatically. |
a34e21d to
bf667db
Compare
Revision history
|
|
Reworked and force-pushed ( The type-exclusion design is out.
What's left is the actual bug and its fix: Kept, since they were asked for directly: the cyan Net −342 lines against the previous revision. |
There was a problem hiding this comment.
Pull request overview
This PR improves the usability and performance of mergify events by bounding the default fetch to a single API page (100 events), while making the output more informative (PR subject per line, clearer summaries, and outcome-based coloring) and explicitly signaling truncation in both human and JSON output.
Changes:
- Default
--limitto 100 (one API page) and reject--limit 0as a usage error. - Add explicit truncation reporting (
truncated) from the fetch layer and surface it in both human header text and--json. - Improve human timeline readability: add PR column (repo-wide), better per-type summaries, and outcome-based coloring via the theme.
Reviewed changes
Copilot reviewed 7 out of 8 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| crates/mergify-tui/src/theme.rs | Changes color enablement policy to depend on a recorded CLI color choice (via set_color_choice) rather than cfg!(test). |
| crates/mergify-events/src/queue_leave.rs | Adapts to the new fetch return type (Log) by reading log.events. |
| crates/mergify-events/src/list.rs | Implements default limit behavior, truncation-aware header/JSON, PR column, improved summaries, and outcome coloring; expands tests accordingly. |
| crates/mergify-events/src/client.rs | Changes fetch to return { events, truncated } and observes truncation based on pagination/overshoot rather than inferring from len == limit. |
| crates/mergify-events/Cargo.toml | Adds direct anstyle dependency for style handling in the events renderer/tests. |
| crates/mergify-cli/src/snapshots/mergify__tests__cli_schema_golden.snap | Updates CLI schema snapshot for events --limit default and help text changes. |
| crates/mergify-cli/src/main.rs | Makes --limit non-optional with a default, adds validation rejecting zero, and updates related tests/docs. |
| Cargo.lock | Records the added anstyle dependency for mergify-events. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
`mergify events` with no flags took minutes on a busy repository:
`--since` defaulted to 24h but `--limit` defaulted to `None`, so the
command walked every page of the window — 100 events at a time —
before printing a line.
`--limit` now defaults to 100, which is exactly one page, so the
no-flag invocation makes one request and stops. The header says when
that cap bit and names the flag that reaches further back:
Mergifyio/monorepo · newest 100 events · 2026-08-09 12:24 → 2026-08-10 12:24 UTC
Older events in this window were not fetched — raise --limit.
That announcement is what makes a default cap acceptable rather than
a way of hiding events, and `--json` carries the same fact as
`truncated`, so a script reading `size` is never quietly reading a
default. `--limit 0` is now a usage error: it would fetch a page and
print none of it, which is indistinguishable from an empty window.
This command is a low-level query over the activity log API. Deciding
*which* events are worth showing is the caller's job, and `--pr` and
`--type` already do it — so the only thing the default owes is a
bound.
Two things the same page was missing, and they are the reason
capping it is not enough:
**A subject on every line.** `Event::pull_request()` was decoded and
never read, so the repo-wide view — the default — never said which
pull request an event was about. It is now a column, cyan `#N` to
match `queue status`, suppressed under `--pr` where the header
already said it. The summary column also learned the types the page
is actually made of, measured over 500 consecutive events on
Mergifyio/monorepo rather than guessed: the queue state pair reports
the queue and its depth, `action.label` prints `+conflict` instead of
"Rule: label on unresolved" for the twentieth time, `action.comment`
prints its first line, `action.request_reviews` the reviewers.
**Color.** `mergify_tui::Theme` already had the palette and the
policy; `list.rs` used `dim` on every column, so `success` and
`failure` were the same grey. The outcome now carries its verdict —
green merged/success, red dequeued/failure, yellow aborted/pending —
and nothing else is colored, because coloring everything reproduces
the wall in a brighter palette.
2026-08-10
10:58 #38425 action.label -review threads unresolved
11:57 #38237 action.queue.leave merged
12:16 #38414 action.queue.leave merged
`truncated` is observed by the fetch rather than inferred from
`events.len() == limit`, which called a window holding exactly `limit`
events truncated.
Before: minutes, indistinguishable lines. After: ~2s on the monorepo.
Refs MRGFY-8533.
Change-Id: I901a6978c85e9c4bf442ac4c3303b6de1da3be87
bf667db to
0b3ce00
Compare
|
Force-pushed ( Two things:
|
Merge Queue Status
This pull request spent 24 seconds in the queue, including 5 seconds running CI. Required conditions to merge
|
mergify eventswith no flags took minutes on a busy repository:--sincedefaulted to 24h but--limitdefaulted toNone, so thecommand walked every page of the window — 100 events at a time —
before printing a line.
--limitnow defaults to 100, which is exactly one page, so theno-flag invocation makes one request and stops. The header says when
that cap bit and names the flag that reaches further back:
That announcement is what makes a default cap acceptable rather than
a way of hiding events, and
--jsoncarries the same fact astruncated, so a script readingsizeis never quietly reading adefault.
--limit 0is now a usage error: it would fetch a page andprint none of it, which is indistinguishable from an empty window.
This command is a low-level query over the activity log API. Deciding
which events are worth showing is the caller's job, and
--prand--typealready do it — so the only thing the default owes is abound.
Two things the same page was missing, and they are the reason
capping it is not enough:
A subject on every line.
Event::pull_request()was decoded andnever read, so the repo-wide view — the default — never said which
pull request an event was about. It is now a column, cyan
#Ntomatch
queue status, suppressed under--prwhere the headeralready said it. The summary column also learned the types the page
is actually made of, measured over 500 consecutive events on
Mergifyio/monorepo rather than guessed: the queue state pair reports
the queue and its depth,
action.labelprints+conflictinstead of"Rule: label on unresolved" for the twentieth time,
action.commentprints its first line,
action.request_reviewsthe reviewers.Color.
mergify_tui::Themealready had the palette and thepolicy;
list.rsuseddimon every column, sosuccessandfailurewere the same grey. The outcome now carries its verdict —green merged/success, red dequeued/failure, yellow aborted/pending —
and nothing else is colored, because coloring everything reproduces
the wall in a brighter palette.
truncatedis observed by the fetch rather than inferred fromevents.len() == limit, which called a window holding exactlylimitevents truncated.
Before: minutes, indistinguishable lines. After: ~2s on the monorepo.
Refs MRGFY-8533.