Skip to content

fix: preserve collector rows and schema across recovery - #594

Closed
simone-chen wants to merge 2 commits into
simonec/vllm031-b200-campaign-v2from
simonec/collector-resume-row-recovery
Closed

simone-chen wants to merge 2 commits into
simonec/vllm031-b200-campaign-v2from
simonec/collector-resume-row-recovery

Conversation

@simone-chen

@simone-chen simone-chen commented Oct 26, 2026 •

Copy link
Copy Markdown
Contributor

Problem and change

  • An interrupted snapshot can retain a CSV row before its callback reaches the checkpoint. Resume replays that callback; first finalization previously persisted both measurements when no Parquet target existed. Apply the existing latest-row policy on first finalization while preserving raw evidence and current-row attestation.
  • Pandas 3 can widen Arrow string keys to large_string during a compatible merge. The next merge then mistakes its own output for an incompatible dataset and replaces earlier rows. Preserve the validated, metric-reconciled Arrow schema and metadata through conversion.
  • Keep measurement identities, newest-attempt selection, MoE eligibility inheritance, metric reconciliation, and genuine incoming schema-mismatch behavior unchanged. Explicit merge_existing=false still overwrites.

Validation

  • 60 finalization tests passed with pandas 3.0.2 and PyArrow 24.0.0. New regressions cover three repeated merges with disjoint/replaced keys, nullable identities, both string widths, field/schema metadata, and strict incompatible-type handling. Both schema-preservation tests fail before this fix.
  • 270 provenance recovery tests passed. Ruff lint/format and diff checks passed.
  • The original 12-row MSA CSV remains 12 → 12 → 12 through the public finalizer with merging enabled, including a final one-row update. Direct merges also retain all rows for both string widths and keep a deliberately slower latest measurement.
  • No GPU execution or timing contract change. Exact-head Full CI is requested after this signed update.

Stack

  • Draft-only stack on the B200 campaign branch. Merge remains gated on review, exact-head Full CI, and a suitable main-targeted integration; no default promotion is included.

Signed-off-by: Simone Chen <simonec@nvidia.com>
@coderabbitai

coderabbitai Bot commented Oct 26, 2026

Copy link
Copy Markdown

Important

Draft PR not reviewed

Draft PRs are not automatically reviewed by default.

  • Trigger a manual review

To automatically review draft PRs, update your CodeRabbit configuration:

reviews:
  auto_review:
    drafts: true
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Comment @coderabbitai help to get the list of available commands.

@simone-chen

Copy link
Copy Markdown
Contributor Author

/ok to test 4b73dc1

Signed-off-by: Simone Chen <simonec@nvidia.com>
@simone-chen simone-chen changed the title fix: deduplicate replayed rows on first finalization fix: preserve collector rows and schema across recovery Oct 26, 2026
@simone-chen

Copy link
Copy Markdown
Contributor Author

/ok to test 6ca136f

@simone-chen

Copy link
Copy Markdown
Contributor Author

Closing as outside the requested review scope: retain only attention #604, recurrent/MoE #605, and communication #591 for Tianhao's review. This campaign infrastructure change is not merged or included in the replacements. Its branch and history remain available for separate future work; collection remains stopped.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant