Context
Deferred from PR #1330 (head ffb4e260b8da480814ca7679f65d847e21010fda), review round 2, finding 1.
Severity as reviewed: LOW-MEDIUM. Not a regression: origin/master behaves the same or worse in the identical scenario.
Location
hypaware-core/plugins-workspace/github/src/cursors.js:26 (MAX_BOUNDARY_IDS = 1000) and cursors.js:159-164 (readBoundaryIds cap)
hypaware-core/plugins-workspace/github/src/capture.js:738 (openGate's ids getter, same slice(0, MAX_BOUNDARY_IDS))
Observed behavior
The boundary gate carries the event ids sitting exactly on a pass's published watermark so an inclusive since cannot re-append them. The carried set is capped at 1000 ids, symmetrically on write and read. When more than 1000 items share one watermark second (a bulk label or milestone sweep, or a history rewrite that restamps thousands of commit dates), the overflow sits outside the guard. slice(0, 1000) keeps insertion order and the published ids are re-seeded first on the next tick, so the same overflow is never absorbed. Reviewer probe at the PR head, with the request budget raised to isolate the effect:
N=1500: tick1 1500 commit rows, 1500 listCommitFiles boundary=1000
tick2 500 commit rows, 500 listCommitFiles boundary=1000
tick3 500 ... tick4 500 ... no convergence
N=900: tick1 900 rows tick2/3/4 0 rows boundary=900
Under the production 400-request budget, 500 listCommitFiles calls per tick means the repository never finishes its commits phase and starves the other repositories in rotation, in addition to duplicate github_events and commit_file rows.
Why deferring is safe
On origin/master the identical scenario re-appends all 1500 items every tick and starves the budget the same way, with no cap at all. The PR strictly improves the case and fully fixes #1284 for every boundary set that fits the cap. The trigger needs more than 1000 items stamped on one wall-clock second, which normal repository activity does not produce.
The reason it was left unfixed in the PR: the only exits are accepting a bounded loss (advance the watermark past a second that will not fit, losing items added to that second after the read) or accepting an unbounded sidecar. Both fork against the anti-loss trade the gate's doc settles (the trade pullChangedSince also refuses), so the choice needs an LLP design decision, not a review-rung repair. The code comment at cursors.js:145-155 records the loop honestly.
Acceptance condition
An LLP records the chosen exit (bounded loss, unbounded sidecar, or an explicit alternative such as absorbing the overflow across ticks), and with the chosen design in place, the reviewer's probe scenario (1500 commits sharing one committer second, budget not confounding) converges: some tick K appends the remaining rows once and every tick after K appends zero rows and spends zero listCommitFiles requests, with no item published in that second after the first read being silently lost unless the LLP explicitly accepts that loss.
Context
Deferred from PR #1330 (head
ffb4e260b8da480814ca7679f65d847e21010fda), review round 2, finding 1.Severity as reviewed: LOW-MEDIUM. Not a regression:
origin/masterbehaves the same or worse in the identical scenario.Location
hypaware-core/plugins-workspace/github/src/cursors.js:26(MAX_BOUNDARY_IDS = 1000) andcursors.js:159-164(readBoundaryIdscap)hypaware-core/plugins-workspace/github/src/capture.js:738(openGate'sidsgetter, sameslice(0, MAX_BOUNDARY_IDS))Observed behavior
The boundary gate carries the event ids sitting exactly on a pass's published watermark so an inclusive
sincecannot re-append them. The carried set is capped at 1000 ids, symmetrically on write and read. When more than 1000 items share one watermark second (a bulk label or milestone sweep, or a history rewrite that restamps thousands of commit dates), the overflow sits outside the guard.slice(0, 1000)keeps insertion order and the published ids are re-seeded first on the next tick, so the same overflow is never absorbed. Reviewer probe at the PR head, with the request budget raised to isolate the effect:Under the production 400-request budget, 500
listCommitFilescalls per tick means the repository never finishes its commits phase and starves the other repositories in rotation, in addition to duplicategithub_eventsandcommit_filerows.Why deferring is safe
On
origin/masterthe identical scenario re-appends all 1500 items every tick and starves the budget the same way, with no cap at all. The PR strictly improves the case and fully fixes #1284 for every boundary set that fits the cap. The trigger needs more than 1000 items stamped on one wall-clock second, which normal repository activity does not produce.The reason it was left unfixed in the PR: the only exits are accepting a bounded loss (advance the watermark past a second that will not fit, losing items added to that second after the read) or accepting an unbounded sidecar. Both fork against the anti-loss trade the gate's doc settles (the trade
pullChangedSincealso refuses), so the choice needs an LLP design decision, not a review-rung repair. The code comment atcursors.js:145-155records the loop honestly.Acceptance condition
An LLP records the chosen exit (bounded loss, unbounded sidecar, or an explicit alternative such as absorbing the overflow across ticks), and with the chosen design in place, the reviewer's probe scenario (1500 commits sharing one committer second, budget not confounding) converges: some tick K appends the remaining rows once and every tick after K appends zero rows and spends zero
listCommitFilesrequests, with no item published in that second after the first read being silently lost unless the LLP explicitly accepts that loss.