Skip to content

Commit 901b26e

Browse files
committed
fix(pm): close-cards — walk the whole timeline, never one page of it
Measured while dry-running the first batch this script was written for: of those 90 cards, #13799 carries more than 100 timeline events, so the single `?per_page=100` read returned a truncated history at HTTP 200 with nothing saying so — and a cross-reference on page 2 read exactly like no cross-reference at all, which is the open-PR skip answering "no" for a card that has one. `readTimeline` now pages by NUMBER until a short page (the spelling the channel table prescribes, cursor exhaustion having been measured to stop early), and a card still returning full pages at the 30-page cap STOPS the run rather than deciding on what it managed to read. The fake board pages for real, so the truncation case is driven rather than modelled. Claude-Session: https://claude.ai/code/session_012GcsUbuqFGBibkEDMRC1eE Co-authored-by: Claude <noreply@anthropic.com>
1 parent 5e6ee19 commit 901b26e

1 file changed

Lines changed: 91 additions & 10 deletions

File tree

‎scripts/pm/close-cards.mjs‎

Lines changed: 91 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -85,6 +85,17 @@
8585
* skipping a card on an unread signal and closing a card on an unread signal
8686
* are both verdicts taken from nothing.
8787
*
88+
* ⛔ And ONE page is not the timeline. Measured on this tree while this script
89+
* was being written: of the 90 cards in the first batch it was built for, one
90+
* (#13799) carries more than 100 timeline events, so a single
91+
* `?per_page=100` read returns a truncated history at HTTP 200 with nothing
92+
* saying so — and a cross-reference on page 2 reads exactly like no
93+
* cross-reference at all. The walk below therefore pages by NUMBER until a
94+
* short page (the spelling `references/rest-channel.md` prescribes, cursor
95+
* exhaustion having been measured to stop early on this platform), and a card
96+
* whose timeline is still not exhausted at `TIMELINE_PAGE_CAP` pages STOPS the
97+
* run rather than deciding on what it managed to read.
98+
*
8899
* ## The three writes, in order, and the one state that must never be left
89100
*
90101
* Per actionable card, in this order:
@@ -163,6 +174,13 @@ export const STEPS = Object.freeze(['comment', 'label', 'close']);
163174

164175
const DEFAULT_EXPECT_STATE = 'pm:queue';
165176

177+
/**
178+
* How many 100-event timeline pages one card may take before the walk refuses.
179+
* ⛔ Not a paging convenience: it is the point at which "I have not finished
180+
* reading" must stop being reported as "I read it all and found nothing".
181+
*/
182+
export const TIMELINE_PAGE_CAP = 30;
183+
166184
const render = (values) => (values.length ? values.map((v) => `\`${v}\``).join(', ') : 'none');
167185

168186
// ---------------------------------------------------------------------------
@@ -363,6 +381,33 @@ export function openPrReferences(timeline) {
363381
return hits;
364382
}
365383

384+
/**
385+
* Every timeline event on a card, walked by PAGE NUMBER until a short page.
386+
*
387+
* ⛔ The completeness of this read is the whole value of the open-PR skip: a
388+
* truncated timeline answers "no open PR references it" for a card that has
389+
* one, at HTTP 200, with no header or field distinguishing it from a complete
390+
* read. So the walk has exactly three outcomes and no fourth — exhausted,
391+
* refused by the transport, or NOT FINISHED — and the third is reported rather
392+
* than rounded down to the second page it did manage to read.
393+
*
394+
* ⛔ Page NUMBERS, not the `Link: rel="next"` cursor: cursor exhaustion has
395+
* been measured on this platform to stop short of the real total, and this
396+
* repo's channel table prescribes the page walk for that reason.
397+
*/
398+
export async function readTimeline(call, base, { pageSize = 100, cap = TIMELINE_PAGE_CAP } = {}) {
399+
const events = [];
400+
for (let page = 1; page <= cap; page++) {
401+
const res = await call(`${base}/timeline?per_page=${pageSize}&page=${page}`, {});
402+
const verdict = classifyHttp({ status: res.status, op: 'card-read', rateRemaining: res.rateRemaining });
403+
if (verdict !== 'ok') return { ok: false, reason: 'transport', verdict, res, pages: page };
404+
const rows = Array.isArray(res.json) ? res.json : [];
405+
events.push(...rows);
406+
if (rows.length < pageSize) return { ok: true, events, pages: page };
407+
}
408+
return { ok: false, reason: 'not-exhausted', verdict: 'prerequisite', events, pages: cap };
409+
}
410+
366411
/**
367412
* Why this card is left alone, or `null` when it is actionable. The order is
368413
* the order the order was written in, and the FIRST reason is the one reported:
@@ -549,18 +594,22 @@ export async function runCloseCards(options, deps = {}) {
549594
// requests rather than 180.
550595
let why = skipReason(card, { expectState, openPrs: [] });
551596
if (!why && skipPrReferenced) {
552-
const timeline = await call(`${base}/timeline?per_page=100`, {});
553-
const timelineVerdict = classifyHttp({ status: timeline.status, op: 'card-read', rateRemaining: timeline.rateRemaining });
554-
if (timelineVerdict !== 'ok') {
555-
record(`#${issue} COULD NOT READ THE TIMELINE — ${timeline.call} -> HTTP ${timeline.status}${timeline.detail ? ` (${timeline.detail})` : ''}`);
597+
const timeline = await readTimeline(call, base);
598+
if (!timeline.ok) {
599+
record(
600+
timeline.reason === 'not-exhausted'
601+
? `#${issue} TIMELINE NOT EXHAUSTED — still full pages at the ${timeline.pages}-page cap (${timeline.events.length} events read).`
602+
: `#${issue} COULD NOT READ THE TIMELINE — ${timeline.res.call} -> HTTP ${timeline.res.status}${timeline.res.detail ? ` (${timeline.res.detail})` : ''}`,
603+
);
556604
record(
557605
'⛔ STOPPING. The open-PR skip is ON, so this card cannot be judged — and closing it on an unread\n' +
558-
' signal is the same act as skipping it on one. Re-run when the route is back, or declare\n' +
559-
' `--no-skip-pr-referenced` if the reading is genuinely not wanted.',
606+
' signal is the same act as skipping it on one. A page of a timeline is not the timeline: a\n' +
607+
' cross-reference on the page nobody read is indistinguishable from none. Re-run when the route\n' +
608+
' is back, or declare `--no-skip-pr-referenced` if the reading is genuinely not wanted.',
560609
);
561-
return result(timelineVerdict === 'refusal' ? EXIT_PLATFORM_REFUSAL : EXIT_PREREQUISITE, { stoppedAt: issue });
610+
return result(timeline.verdict === 'refusal' ? EXIT_PLATFORM_REFUSAL : EXIT_PREREQUISITE, { stoppedAt: issue });
562611
}
563-
why = skipReason(card, { expectState, openPrs: openPrReferences(timeline.json) });
612+
why = skipReason(card, { expectState, openPrs: openPrReferences(timeline.events) });
564613
}
565614

566615
if (why) {
@@ -651,13 +700,14 @@ const SELF_TEST_BATTERIES = Object.freeze({
651700
'the pre-flight: a closing comment no card should receive': 7,
652701
'the skip matrix: every reason a card is left alone': 14,
653702
'the open-PR reading: a cross-reference that is a PR, and open': 7,
703+
'the timeline walk: one page is not the timeline': 8,
654704
'the happy path: three writes per card, in order': 10,
655705
'the half-write refusal: stop at the first card left in a state nobody asked for': 12,
656706
'the unreadable card: a verdict taken from nothing is not taken': 7,
657707
'the dry run: a plan that proves it wrote nothing': 7,
658708
'the sibling tools: driven, never re-implemented': 8,
659709
});
660-
const SELF_TEST_BATTERY_FLOOR = 10;
710+
const SELF_TEST_BATTERY_FLOOR = 11;
661711
const UNATTRIBUTED_BATTERY = '(unattributed)';
662712

663713
const batteryCases = new Map();
@@ -705,7 +755,16 @@ export function fakeApi(initial = {}) {
705755

706756
const card = cards.get(number);
707757
if (!card) return wrap(404, { message: 'Not Found' });
708-
if (kind === 'timeline') return wrap(200, card.timeline ?? []);
758+
if (kind === 'timeline') {
759+
// Paged for real: a fake that answered the whole timeline to every
760+
// request would pass the walk's assertions while the truncation the walk
761+
// exists for went untested.
762+
const query = new URLSearchParams(path.slice(path.indexOf('?') + 1));
763+
const size = Number(query.get('per_page') ?? 100);
764+
const page = Number(query.get('page') ?? 1);
765+
const all = card.timeline ?? [];
766+
return wrap(200, all.slice((page - 1) * size, page * size));
767+
}
709768
if (kind === 'patch') {
710769
card.state = init.body?.state ?? card.state;
711770
card.state_reason = init.body?.state_reason ?? card.state_reason;
@@ -834,6 +893,28 @@ export async function selfTest() {
834893
t('⛔ a non-array timeline reads as no references, never as a crash', openPrReferences(null), []);
835894
t('several open PRs are all reported', openPrReferences([CROSS_REF(1, 'open'), CROSS_REF(2, 'closed'), CROSS_REF(3, 'open')]), [1, 3]);
836895

896+
// ── the timeline walk ─────────────────────────────────────────────────────
897+
battery('the timeline walk: one page is not the timeline');
898+
const BASE = '/repos/objectstack-ai/objectstack/issues/1';
899+
const noise = (n) => Array.from({ length: n }, () => ({ event: 'labeled', label: { name: 'tooling' } }));
900+
const oneShort = fakeApi({ cards: { 1: { state: 'open', timeline: noise(3) } } });
901+
const short = await readTimeline(oneShort.call, BASE, { pageSize: 100 });
902+
t('a short first page ends the walk in one request', [short.ok, short.pages], [true, 1]);
903+
t('…and returns every event on it', short.events.length, 3);
904+
const twoPages = fakeApi({ cards: { 1: { state: 'open', timeline: [...noise(100), CROSS_REF(777, 'open')] } } });
905+
const walked = await readTimeline(twoPages.call, BASE, { pageSize: 100 });
906+
t('a FULL page is followed by the next one', [walked.ok, walked.pages], [true, 2]);
907+
t('…and a cross-reference on page 2 is SEEN — the measured truncation this walk closes', openPrReferences(walked.events), [777]);
908+
const dead = fakeApi({ cards: { 1: { state: 'open', timeline: noise(300) } }, hooks: { timeline: [{ status: 200, json: noise(100) }, { status: 502, json: { message: 'Bad gateway' } }] } });
909+
const broke = await readTimeline(dead.call, BASE, { pageSize: 100 });
910+
t('⛔ a transport failure mid-walk is a refusal, never a short page', [broke.ok, broke.reason], [false, 'transport']);
911+
const huge = fakeApi({ cards: { 1: { state: 'open', timeline: noise(50) } } });
912+
const capped = await readTimeline(huge.call, BASE, { pageSize: 1, cap: 4 });
913+
t('⛔ a timeline still full at the cap is NOT EXHAUSTED, not "nothing found"', [capped.ok, capped.reason], [false, 'not-exhausted']);
914+
t('…and it reports how far it got, so the refusal can say so', [capped.pages, capped.events.length], [4, 4]);
915+
const buried = await driveOffline({ numbers: [91] }, { cards: { 91: { state: 'open', labels: ['pm:queue'], assignees: [], timeline: [...noise(100), CROSS_REF(778, 'open')] } } });
916+
t('a card whose only open-PR reference sits on page 2 is SKIPPED, not closed', buried.res.counts.skipped, 1);
917+
837918
// ── the happy path ────────────────────────────────────────────────────────
838919
battery('the happy path: three writes per card, in order');
839920
const happy = await driveOffline({ numbers: [11, 12] }, { cards: { ...QUEUED(11), ...QUEUED(12) } });

0 commit comments

Comments
 (0)