Skip to content

fix(demo): wait for full ledger coverage before log dump in FVV demo#1433

Open
sei-ahouseholder wants to merge 3 commits into
mainfrom
bug/1363-fvv-late-joiner-backfill
Open

fix(demo): wait for full ledger coverage before log dump in FVV demo#1433
sei-ahouseholder wants to merge 3 commits into
mainfrom
bug/1363-fvv-late-joiner-backfill

Conversation

@sei-ahouseholder

Copy link
Copy Markdown
Contributor

Summary

_phase_case_closure in fvv_demo.py waited only for the tail entry
hash before dumping JSONL logs. Announce(CaseLedgerEntry) activities
are delivered independently per-entry; an intermediate entry (e.g.
logIndex=17) can arrive after the tail, so _phase_dump_case_ledgers
captured a gapped log, causing test_invariant_8_late_joiner_has_full_history
to fail intermittently.

Adds wait_for_contiguous_ledger_coverage() to polling.py which polls
until the replica holds all log indices 0…expected_tail_index before
the dump proceeds. Replaces both wait_for_finder_log_entry() call sites
in the FVV demo (Phase 2 sync verification and Phase 6 case closure) with
this stronger gate.

Changes

  • vultron/demo/helpers/polling.py: add wait_for_contiguous_ledger_coverage()
  • vultron/demo/helpers/__init__.py: re-export new helper
  • vultron/demo/scenario/fvv_demo.py: replace tail-hash-only wait with
    contiguous coverage check in _phase_sync_verification and _phase_case_closure
  • test/demo/test_fvv_demo.py: 5 regression tests in TestWaitForContiguousLedgerCoverage

Verification

  • 5 new regression tests — including the exact race (tail present, intermediate entry absent)
  • 4706 unit tests pass (uv run pytest --tb=short)
  • Black, flake8, mypy, pyright all clean

…1363)

Replace tail-hash-only wait with contiguous index coverage check in
_phase_case_closure and _phase_sync_verification.

Announce(CaseLedgerEntry) activities are delivered independently; an
intermediate entry (e.g. logIndex=17) can arrive after the tail entry,
leaving a gap in the JSONL dump captured by _phase_dump_case_ledgers.

Add wait_for_contiguous_ledger_coverage() to polling.py which polls until
the replica holds all log indices 0...expected_tail_index before the dump
proceeds.  Replace both wait_for_finder_log_entry() call sites in the FVV
demo with this stronger gate.

Add 5 regression tests in TestWaitForContiguousLedgerCoverage covering:
- full coverage returns immediately
- missing intermediate index (the exact #1363 race)
- tail present but early entries absent
- entries from other cases excluded
- no entries present raises

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@sei-ahouseholder sei-ahouseholder added the size:M 51-300 diff lines or 3-6 ACs label Jul 14, 2026
ahouseholder and others added 2 commits July 14, 2026 20:12
SYNC-02-003 describes replication originating from the CaseActor, not
the contiguous-coverage pre-dump gate requirement.  SYNC-10-004 is the
correct reference: "catch-up gate MUST require a contiguous canonical
log prefix from genesis through the actor's currently acknowledged tip."

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

@sei-ahouseholder sei-ahouseholder left a comment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

PR Review: #1433 — FVV late-joiner backfill fix

Phase 1 — Issue Linkage: PASS

  • Closes #1363
  • Implementation scope matches the issue exactly: replace tail-hash-only wait
    with full contiguous index coverage.

Phase 2 — PR Body Format: PASS

  • Closing reference at top ✓
  • Summary, Changes, Verification sections present ✓
  • Test count (5 new) is a real number ✓
  • Co-authored-by trailer present in the fix commit ✓

Phase 3 — Domain Context / Spec Conformance: FIXED

The new wait_for_contiguous_ledger_coverage() docstring originally cited
SYNC-02-003 — but that spec governs replication originating from the
CaseActor, not the contiguous-coverage pre-dump gate. The correct reference
is SYNC-10-004 ("catch-up gate MUST require a contiguous canonical log
prefix from genesis").

FIXED (commit c9644a27): corrected docstring spec reference to
SYNC-10-004.

Phase 4 — ADR Check: PASS

No new architectural layers, protocols, or public API changes; no ADR required.

Phase 5 — AGENTS.md Compliance: PASS

Demo helper changes only; no BT nodes, use cases, or core domain touched.

Phase 6 — Code Review: PASS

  • wait_for_contiguous_ledger_coverage builds expected_indices once before
    the _check closure (efficient set comparison). ✓
  • Uses _poll_until generic primitive consistently with all other helpers. ✓
  • Filters by case_id to avoid counting entries from other cases. ✓
  • Logs missing indices (up to 10) at DEBUG for diagnosability. ✓
  • fvv_demo.py: both Phase 2 and Phase 6 sites updated; the Phase 2 site
    also correctly checks Vendor2 in addition to Finder. ✓

Phase 7 — Notes Currency: PASS

notes/sync-ledger-replication.md covers the design but doesn't need
updating — the change is a demo-layer polling improvement, not an
architecture change.

Phase 8 — Test Coverage: PASS

5 regression tests in TestWaitForContiguousLedgerCoverage cover:

  1. Happy path (all indices present) ✓
  2. Missing intermediate index — exact #1363 race ✓
  3. Tail present but early entries absent ✓
  4. Entries from other cases excluded ✓
  5. No entries present ✓

Phase 9 — CI Status: PASS

All 12 checks pass, including FVV Demo Integration and Two-Actor Demo Integration. ✓

DEFER

#1434two_actor_demo.py still uses wait_for_finder_log_entry()
(tail-hash-only) at lines 574 and 770. The same race applies to the
two-actor demo. Tracked in follow-up issue #1434; out of scope for this
PR since the bug report was FVV-specific and two-actor CI is currently green.


Overall: ready to merge. Root cause fixed, 5 regression tests, one
inaccurate spec reference corrected in commit c9644a27.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size:M 51-300 diff lines or 3-6 ACs

Projects

None yet

Development

Successfully merging this pull request may close these issues.

bug: FVV demo occasionally fails test_invariant_8_late_joiner_has_full_history (intermittent backfill miss)

2 participants