fix: comprehensive per-progress heartbeat coverage audit across worker/indexer long operations (#229)#244
Conversation
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: e500c79470
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| request_key, | ||
| requested_chain_positions, | ||
| topology_version_boundary, | ||
| record_version_boundary | ||
| FROM execution_cache_outcomes | ||
| WHERE request_type IN ('verified_resolution', 'verified_primary_name') | ||
| AND ($1::text IS NULL OR execution_cache_key > $1) | ||
| WHERE ($1::text IS NULL OR execution_cache_key > $1) |
There was a problem hiding this comment.
When execution_cache_outcomes contains many rows for request types that reorg repair does not invalidate, such as declared_resolution, this query now pages the whole table and only skips those rows later in Rust. The removed request_type IN ('verified_resolution', 'verified_primary_name') predicate let the database discard out-of-scope rows before LIMIT, so each batch advanced through actual invalidation candidates instead of potentially many zero-work pages; please keep that predicate in SQL while still selecting request_type if needed for decoding.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Dispositioned: confirmed and fixed. R1 fixed in 7692b26 — the request_type predicate is restored in SQL (request_type still selected for decoding); a dominant-out-of-scope fixture asserts the expected batch progression.
| return generic_topic_scan_source_identity_payload(source_plan); | ||
| } | ||
|
|
||
| if source_plan.selected_targets.len() <= COMPACT_SOURCE_IDENTITY_SELECTED_TARGET_THRESHOLD { |
There was a problem hiding this comment.
For whole-active or watched-target-set plans with more than 10,000 selected targets, this now falls through into the custom compact payload instead of delegating to WatchedSourceSelectorPlan::source_identity_payload(). That changes the persisted source_identity_hash (for whole-active plans it switches away from the shared FNV identity, and for watched-target sets it drops the full selected target list), so existing bootstrap checkpoints/backfill jobs won't be found and new jobs can be minted for already-covered work. Please keep this compaction limited to the source-family case and mirror that in the progress helper.
AGENTS.md reference: AGENTS.md:L8-L8
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Dispositioned: confirmed and fixed. R2 fixed in dea721e — compaction is again strictly source-family-scoped; whole-active and watched-target-set identity payloads/hashes are byte-stable vs origin/main (golden-hash tests: fnv1a64 whole-active, full-target-list watched sets), mirrored in the progress helper.
| })?; | ||
| let parent_hash = block.parent_hash.clone(); | ||
| let snapshots = | ||
| mark_chain_lineage_range_orphaned(pool, chain, &block_hash, parent_hash.as_deref()) |
There was a problem hiding this comment.
When a reorg losing branch has more than one block, this loop now commits one-block chain_lineage orphaning transactions before the rest of the branch has been validated or updated. If a later parent lookup, orphan update, or heartbeat fails, the checkpoint still references the old head while one or more blocks on that path are already marked orphaned, leaving canonicality partially repaired; the previous range call loaded and updated the whole losing branch in one transaction. Please keep the branch canonicality update atomic, or validate the full path before doing per-block heartbeats.
AGENTS.md reference: AGENTS.md:L23-L23
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Dispositioned: confirmed and fixed. R3 fixed in 5a76ddf — the losing branch is validated (with per-block beats) first, then chain_lineage is updated in ONE transaction; an injected middle-block failure test leaves every block canonical, no partial repair.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 5a76ddf4d1
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| let mut after_id = 0i64; | ||
| loop { | ||
| let ids = sqlx::query_scalar::<_, i64>( | ||
| "SELECT normalized_event_id FROM normalized_events WHERE normalized_event_id > $1 ORDER BY normalized_event_id LIMIT $2", |
There was a problem hiding this comment.
When a retained semantic witness check runs for one chain, this id page now advances through every normalized_events row in the database before the follow-up query narrows to ENSv2 registry raw-log events for that chain. On mainnet-sized tables this can turn each startup verification into a global normalized-event table walk with many zero-work pages; please include the chain/derivation/raw-fact/canonicality filters in the paged query before applying LIMIT.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Dispositioned: confirmed and fixed — witnesses id page now carries the chain/derivation/raw-fact/canonicality filters before LIMIT (aeb2ccd). Additionally, a full sweep enumerated every paged query this branch introduced or restructured; each is now either fixed or verified to carry its scoping predicates before LIMIT (table in the PR record).
| SELECT discovery_edge_id | ||
| FROM discovery_edges | ||
| WHERE discovery_edge_id > $1 | ||
| ORDER BY discovery_edge_id | ||
| LIMIT $2 |
There was a problem hiding this comment.
When reconciling a single discovery source, this page source walks all discovery_edges rows and only the follow-up query filters to active rows for de.discovery_source = $1. On deployments with many edges from other sources, a source-specific full reconcile can spend most pages doing zero deactivation work before reaching its own candidates; please page the active rows for the requested source before applying LIMIT.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Dispositioned: confirmed and fixed — streamed single-source deactivation now pages active rows for the requested discovery_source before LIMIT (aeb2ccd). Additionally, a full sweep enumerated every paged query this branch introduced or restructured; each is now either fixed or verified to carry its scoping predicates before LIMIT (table in the PR record).
| source_scope: Option<&[(String, String, i64, i64)]>, | ||
| mode: PersistedRawPayloadAdapterSyncMode, | ||
| reconcile_full_source: bool, | ||
| progress: &mut Option<&mut dyn bigname_adapters::StartupAdapterProgress>, |
There was a problem hiding this comment.
When a progress-enabled block-hash adapter sync reaches the ENSv2 registry outside the full-source/live-poll arms, this new progress handle is not used: the LiveOrBackfill arms below still call the non-progress sync_for_block_hashes* entrypoints. A large startup or post-bootstrap chunk can therefore actively page and persist ENSv2 registry work without refreshing the indexer service heartbeat until the adapter returns; please add and use progress-aware block-hash entrypoints when progress is present.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Dispositioned: confirmed and fixed — progress-aware block-hash entrypoints added and routed in the LiveOrBackfill arms when progress is present; the coverage map's disposition row updated (2389386). Additionally, a full sweep enumerated every paged query this branch introduced or restructured; each is now either fixed or verified to carry its scoping predicates before LIMIT (table in the PR record).
| let source_ids = sqlx::query_scalar::<_, i64>(&format!( | ||
| "SELECT {source_id_column} FROM {source_table} WHERE {source_id_column} > $1 ORDER BY {source_id_column} LIMIT $2" | ||
| )) |
There was a problem hiding this comment.
When stored-lineage coverage publication runs with progress, this page source first walks every row in contract_instance_addresses or discovery_edges, and only the follow-up insert filters to the requested chain, source families, and block range. On deployments with many unrelated addresses or edges, publishing one chain's coverage frontier can spend most pages doing no candidate work while repeatedly rebuilding watched CTEs for global pages; please apply the watched-scope filters before LIMIT.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Dispositioned: confirmed and fixed — coverage-frontier page sources now apply the watched chain/family/intersection scope before LIMIT (aeb2ccd). Additionally, a full sweep enumerated every paged query this branch introduced or restructured; each is now either fixed or verified to carry its scoping predicates before LIMIT (table in the PR record).
2389386 to
5057793
Compare
There was a problem hiding this comment.
💡 Codex Review
Thread replay recovery through progress
When automatic normalized replay hits missing ENSv2 retained-history coverage, this branch still calls the non-progress recovery helper even though progress is in scope. That helper runs the hash-pinned provider backfill without run_resumable_hash_pinned_backfill_job_with_progress, so an exact recovery range large enough to take longer than the heartbeat max age can leave the spawned catch-up task’s service-loop heartbeat stale despite active work. Please route the Some(progress) case through recover_ens_v2_live_coverage_requirement_with_progress, as the live-poll recovery path does.
AGENTS.md reference: AGENTS.md:L7-L7
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01C9RTnbCyhqEUSPc7xCK76p
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01C9RTnbCyhqEUSPc7xCK76p
Keep heartbeat reporting per candidate batch while restricting keyset pagination to verified resolution and verified primary-name outcomes in SQL. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01C9RTnbCyhqEUSPc7xCK76p
Limit the progressive compact identity to source-family plans and retain the pre-existing whole-active and watched-target-set payloads and hashes while recording heartbeat progress. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01C9RTnbCyhqEUSPc7xCK76p
Validate the losing lineage path with progress heartbeats before applying one transactional range update, so failures cannot leave mixed canonicality. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01C9RTnbCyhqEUSPc7xCK76p
Apply operation scope before LIMIT across retained witnesses, discovery reconciliation, watched coverage, resolver convergence, and related audit sweep sites. Add dominated-table exact-page-count regressions and correct the exact-boundary requirement beat. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01C9RTnbCyhqEUSPc7xCK76p
Add progress-aware ENSv2 registry block-hash entrypoints, route every reconciliation mode through them when a heartbeat handle is present, and lock the coverage with an out-of-scope-dominated exact-beat regression. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01C9RTnbCyhqEUSPc7xCK76p
Route completed bootstrap raw-range replay through the progress-aware normalized-event entrypoint and cover multi-page callbacks at the caller boundary. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01C9RTnbCyhqEUSPc7xCK76p
Restore origin/main's range-transaction boundaries for losing-branch payload families and winning-lineage re-canonicalization while retaining read-side and post-commit progress beats. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01C9RTnbCyhqEUSPc7xCK76p
Use total emitter identity ordering after both watched-contract loader paths and restore the ordinary full-replay page size while retaining small startup heartbeat pages. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01C9RTnbCyhqEUSPc7xCK76p
8f23b7e to
04c08fc
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 04c08fc92f
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| WHERE desired.desired_row_id > $2 | ||
| AND desired.active_from_block_number IS NOT NULL | ||
| AND EXISTS ( | ||
| SELECT 1 | ||
| FROM discovery_edges de | ||
| JOIN contract_instance_addresses cia | ||
| ON cia.contract_instance_id = de.to_contract_instance_id | ||
| AND cia.deactivated_at IS NULL | ||
| WHERE de.discovery_source = $1 | ||
| AND de.deactivated_at IS NULL | ||
| AND de.provenance ->> 'observation_key' = desired.observation_key | ||
| AND de.chain_id = desired.chain_id | ||
| AND de.edge_kind = desired.edge_kind | ||
| AND de.from_contract_instance_id = desired.from_contract_instance_id | ||
| AND NOT {STREAMED_EDGE_IS_ORPHANED_SQL} | ||
| AND {STREAMED_STARTS_AFTER_SQL} | ||
| ) | ||
| ORDER BY desired.desired_row_id | ||
| LIMIT $3 |
There was a problem hiding this comment.
When a streamed full-source reconcile has many staged desired edges but only a small retained-successor diff, this page now advances through every desired row with a start block and only the LATERAL query below decides whether any of those rows are historical. The previous successor EXISTS predicate was applied before LIMIT, so each page advanced through actual historical candidates; without it, a near-no-op finalize can spend most pages doing no historical work while holding the reconciliation transaction. Please put the successor condition back into the paged query, or page a materialized successor-candidate set.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Dispositioned: confirmed and fixed. F1 fixed — the successor condition is restored into the paged query before LIMIT, with a page-progression test on a non-historical-dominated fixture.
| provider: Option<&(impl ChainProviderOps + ?Sized)>, | ||
| header_audit_mode: HeaderAuditMode, | ||
| mut raw_log_input_version: RawLogStagingInputVersion, | ||
| progress: &mut Option<&mut NormalizedReplayHeartbeat>, |
There was a problem hiding this comment.
When automatic normalized replay hits missing ENSv2 coverage, this new progress handle is not propagated: the recovery branch below still calls the non-progress recover_ens_v2_live_coverage_requirement, which runs the hash-pinned recovery backfill without heartbeat callbacks. For a large generation-bound recovery interval, the required replay subtask can actively fetch and persist coverage while the shared indexer heartbeat goes stale; please call the _with_progress recovery path when progress is present.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Dispositioned: confirmed and fixed. F2 fixed — the coverage-recovery branch now routes through the _with_progress recovery path when a progress handle is present, with a per-page beat test. The sweep was also re-run on the final tree (post-rebase) to close both classes again.
Filter staged desired edges by a qualifying active successor before the page limit, and cover page progression with a non-historical-dominated fixture. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01C9RTnbCyhqEUSPc7xCK76p
Route normalized replay ENSv2 recovery, scoped ENSv1 emitter conversion, and startup authority checkpoint deletion through their progress-aware paths. Add caller-level per-page coverage and scoped-emitter regressions. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01C9RTnbCyhqEUSPc7xCK76p
2 participants
Closes #229 — the systematic sweep ending the one-more-un-instrumented-op-per-review-round whack-a-mole. Enumerates every worker + indexer main/spawned-loop operation that can plausibly exceed the 20s heartbeat max-age at mainnet scale, and gives each a disposition: covered (with where), newly-covered (this PR), cannot-exceed-20s (with the scale reasoning), or n/a. The maintained coverage map lives in docs/storage.md's heartbeat section; one-shot operational commands (manual backfill/replay/rewind/repair) are dispositioned n/a as they run outside the registered service loops.
Newly instrumented surfaces (progress-tied beats only — never timers, preserving the false-green discipline): the canonical reconciliation tree, the resolver-profile-convergence tree, projection rebuild/hydration passes, the invalidation apply side, the backfill coordinator, and live-poll manifest/discovery refresh. Prior art (the untested 102-file sweep at 0c1e07c) was mined for locations; every change re-derived cleanly against current main with a beat-fires-during-long-op test per surface. All #213 false-green repro + wedge-stops-beating tests preserved.
Verify-loop found and fixed two real defects in-loop, both with regression tests: a shutdown path blocked by a held replay activity gate, and a deployment pool-minimum mismatch.
Gates
fmt, strict denying clippy, file-size gate (advisory warnings only), DB suites 2,121 passed / 0 failed (API 697, indexer 528, manifests 114, storage 498, worker 284), docs consistency check.
🤖 Generated with Claude Code
https://claude.ai/code/session_01C9RTnbCyhqEUSPc7xCK76p