[groeneai]

Problem to be solved:

GitHubSource.fetch() ran gh api notifications?since=... without --paginate, so it read only the first page (50 threads; per_page is capped at 50), then moved the cursor to the newest updated_at on that page. The next since excluded the unread pages, so they never reached source_messages. On this install a 5.6 h sync gap lost about 40 threads, 7 of them direct asks from maintainers; the store shows 19 full-page runs since June, 14 after gaps under 1.3 h.

The cursor rule has to change too. GitHub orders /notifications newest-first by, and applies since to, the time a thread last notified the user, which it does not return. updated_at also moves on events that do not notify (own pushes, CI, labels, edits): in a live 48 h window, 276 of 497 threads had an updated_at more than 1 h above their place in the listing. So no updated_at watermark is safe inside a window.

Changes:

• gh api ... --paginate --jq '.[]', parsed as JSON Lines split on \n (not splitlines(): gh prints U+0085 raw). A failed listing (rc != 0, timeout, bad line) holds the cursor.
• De-duplicate by id, keeping the later copy: a thread notified mid-listing pushes the boundary item onto the next page.
• The cursor is the first listed item's updated_at, not the max: the first item comes from the first page, so no notification landing while later pages are read is older than it. The next since is the cursor itself (inclusive), so a notification in the cursor's second is not lost.
• insert_source_messages returns the ids it stored, and SourceRunner condenses and counts only those, so the thread re-listed in that second is not condensed again or logged as ingested on every run.

Cost: one run now enriches the whole backlog before it persists, ~15 s per notification (a 114-thread gap ~30 min, ~495 threads ~2 h). A scheduled run meanwhile is skipped by the runner lock; a source reload or a CLI nerve sync reads the window again, which repeats work but cannot lose a thread (each run persists before it writes its cursor).

Not changed: github_events and github_repos also read one page (both disabled by default); I can page them too if wanted. Overlaps open #422: both edit SourceRunner._run_locked, and its test reads insert_source_messages as a count. Residual: a thread marked read during the ~3 s listing shifts the next page up by one and can skip an item; nothing in nerve marks notifications read.

Tests: a fake that models the hidden listing key (_k <= updated_at): a 120-thread backlog with 5 updated_at above every key comes back in full from one call; a notification landing between pages or in the cursor's second comes back from the next call; a raw U+0085 parses; a second run over unchanged records condenses and counts nothing. Live (read-only, 48 h): main misses 448 of 498 threads, this branch 0. pytest tests/: 3934 passed (3929 on main).

🤖 Generated with Claude Code

[groeneai]

Internal second-model review: adjudication log (click to expand)

Pre-publication review by an independent model (engine: codex; 4 rounds, 4 findings) plus my own cold review
(4 rounds, 10 findings). The last round found no new defect.

#	Sev	Finding	Verdict	Evidence / action
1	❌	A notification landing in the cursor's own second is skipped by the next since = cursor + 1s (nerve/sources/github.py fetch)	AGREE, fixed	since is the cursor itself; same-second regression test
2	⚠️	str.splitlines() also splits on U+0085 inside a JSON string, which gh prints raw, so one such notification stalls the source	AGREE, fixed	split on \n only; raw UTF-8 regression test
3	⚠️	The thread re-listed in the cursor's second is condensed again and counted as ingested on every run (SourceRunner._run_locked)	AGREE, fixed	insert_source_messages returns the ids it stored; the runner condenses and counts only those; consecutive-run test
4	⚠️	The runner lock is per instance, so a reload or CLI nerve sync can overlap a long run, contrary to the description	AGREE (description), no code change	an overlap repeats work but cannot lose a thread: each run persists before it writes its cursor (runner.py), the batch insert is atomic; description corrected
5	💡	Test fakes escape non-ASCII (ensure_ascii=True), unlike gh	AGREE, fixed	the paging fake emits raw UTF-8
6	💡	A thread marked read mid-listing shifts the next page up by one	AGREE, not changed	nothing in nerve marks notifications read; stated in the description
7	💡	On the 60 s listing timeout the gh child is not killed	AGREE, not changed	pre-existing, unchanged by this PR
8	💡	A very large backlog (more than ~150 pages, or a run longer than the restart cadence) repeats the window	AGREE, not changed	far above the measured rate; stated in the description
9	⚠️	docs/sources.md still said GitHub advances its cursor by +1s, and listed condensation before persistence	AGREE, fixed	Cursor Design, overview, ingestion list and config table brought in line with the code
10	💡	Open #422 (and #465, which contains it) also edits SourceRunner._run_locked and docs/sources.md, and reads insert_source_messages as a count	AGREE	stated in the description; the docs/sources.md overlap is a text conflict for whichever PR merges second

Severity: ❌ blocker / ⚠️ major / 💡 nit. DISAGREE verdicts carry recorded evidence and are
terminal per finding. Findings on hunks unchanged by the fix round are auto-dropped.

Session id: cron:clickhouse-review-self-slot-58:20261006-141800

[groeneai]

cc @alex-clickhouse, could you review this? It makes the GitHub source read every /notifications page (a sync gap over 50 threads lost the older ones before storage) and changes insert_source_messages to return the stored ids, which overlaps your #422 and #465 in SourceRunner._run_locked and docs/sources.md.

[alex-clickhouse]

LGTM but let's merge after #422

[pufit]

@groeneai please resolve conflicts

[groeneai]

Rebased onto main and resolved the docs/sources.md conflict with #422. Two follow-ups to #422 came with it: test_channel_observation.py now asserts on the stored-id list that insert_source_messages returns, and a779b0f makes the runner count and condense a record id that ChannelSource repeats within one batch once (it counted both copies). Full suite: 6419 passed.

[nerveai]

Nerve PR review · ✅ Ready to merge (once CI is approved)

This fixes real data loss. On main, a sync gap with more than 50 participating notifications drops everything past page 1. Reading the whole window with --paginate and taking the cursor from the first listed item is the right fix for GitHub's hidden listing key. I checked the ordering argument and found no gap. Returning stored ids from storage also stops repeat condense calls for every source.

Completeness: a paging fake models the hidden key, and there are runner tests for stored and repeated ids. 6 of the 7 new tests fail on main. docs/sources.md is updated. No config or migration changes.

Worth considering:

• Overlap with Add a drop hook to the source guardrail #512: the cursor's own second is listed again every run (nerve/sources/github.py:124). Storage skips a kept copy, but a guardrail-dropped one is dropped again every run (nerve/sources/runner.py:347). With Add a drop hook to the source guardrail #512's drop hook, the hook fires on that thread every idle run. Whichever PR merges second should de-duplicate repeat drops.
• After a long gap, nothing persists until the whole backlog is enriched. Fresh asks wait behind it, and a restart mid-run discards that work.

_{Checked: offline sandbox at a779b0f: 6405 passed, 38 skipped. test_slack_integration.py was excluded because it hangs on main e306482 too. CI not yet run (fork approval pending) · automated review by nerveai — mention @nerveai to ask a follow-up.}