Skip to content

Fix #364: [milestone Milestone 6 ] Add examples/divergence_demo.py: load two pre-recorded dumps, invoke `replay_d... - #369

Open
telleroutlook wants to merge 1 commit into
mainfrom
claude/issue-364
Open

Fix #364: [milestone Milestone 6 ] Add examples/divergence_demo.py: load two pre-recorded dumps, invoke `replay_d...#369
telleroutlook wants to merge 1 commit into
mainfrom
claude/issue-364

Conversation

@telleroutlook

Copy link
Copy Markdown
Contributor

Fixes #364

Generated by claude-bot-go worker.

… load two pre-recorded dumps, invoke `replay_d...
@telleroutlook

Copy link
Copy Markdown
Contributor Author

Summary

Internally consistent diff: replay_diff correctly loads two dumps, rank-aligns action streams, and reports the first divergence; tests verify the asserted outputs and the restricted-unpickler round-trip. No blockers.

Severity

low

Blocking findings

  1. train_replay/replay/replayer.py:158 replay_diff uses no instance state — every helper it calls (_load_bundle, _group_actions_by_rank, _first_divergence) is a @staticmethod, so the graph passed to EpochReplayer is unused for this operation. Could be a staticmethod or standalone function; current shape works but couples the API to an instance needlessly.
  2. train_replay/replay/replayer.py:233 The length-mismatch branch (_first_divergence at the len(baseline) != len(candidate) check, which triggers _missing_record sentinel records) is not exercised by any test — tests only cover value divergence and byte-identical dumps, leaving the missing-record path unverified.

Verdict

✅ Approved

Merge risk

low

Audit

  • model: opus
  • effort: high
  • tokens: input=0, output=0
{
  "approved": true,
  "severity": "low",
  "summary": "Internally consistent diff: replay_diff correctly loads two dumps, rank-aligns action streams, and reports the first divergence; tests verify the asserted outputs and the restricted-unpickler round-trip. No blockers.",
  "findings": [
    {
      "file": "train_replay/replay/replayer.py",
      "line": 158,
      "issue": "replay_diff uses no instance state — every helper it calls (_load_bundle, _group_actions_by_rank, _first_divergence) is a @staticmethod, so the graph passed to EpochReplayer is unused for this operation. Could be a staticmethod or standalone function; current shape works but couples the API to an instance needlessly.",
      "kind": "suggestion"
    },
    {
      "file": "train_replay/replay/replayer.py",
      "line": 233,
      "issue": "The length-mismatch branch (_first_divergence at the `len(baseline) != len(candidate)` check, which triggers _missing_record sentinel records) is not exercised by any test — tests only cover value divergence and byte-identical dumps, leaving the missing-record path unverified.",
      "kind": "suggestion"
    }
  ],
  "merge_risk": "low",
  "Model": "opus",
  "model": "claude-cli",
  "effort": "high",
  "tokens": {
    "input_tokens": 0,
    "output_tokens": 0
  },
  "same_verdict_count": 1
}

@telleroutlook

Copy link
Copy Markdown
Contributor Author

Merge gate failed: go test ./... exited non-zero.

verify manifest: /srv/claude-bot/worktrees/WasmAgent_wasmagent-train-replay/.gate-1785127652652920058/.claude-bot/verify.yml
  [PASS] install (SLUG=$(basename "$PWD"); if timeout 8 bash -c "test -d /mnt/bigdisk/venv && touch /mnt/bigdisk/venv/.probe-$SLUG 2>/dev/null && rm -f /mnt/bigdisk/venv/.probe-$SLUG" 2>/dev/null; then VENV=/mnt/bigdisk/venv/$SLUG; echo "[venv] bigdisk: $VENV"; else VENV="$PWD/.venv-local"; echo "[venv] local fallback: $VENV"; fi; { [ -x "$VENV/bin/python" ] || python3 -m venv "$VENV"; } && rm -rf .venv && ln -sfn "$VENV" .venv && .venv/bin/pip install -e ".[dev]" -q --no-cache-dir --extra-index-url https://download.pytorch.org/whl/cpu) took 4m54.534s
  [PASS] python-import (.venv/bin/python -c "import train_replay; import train_replay.agent; import train_replay.replay; import train_replay.recording") took 109ms
  [PASS] lint (.venv/bin/python -m ruff check train_replay tests) took 438ms
  [PASS] typecheck (.venv/bin/python -m mypy train_replay) took 3.705s
  [FAIL] test (.venv/bin/python -m pytest tests/ -x) took 705ms

Failing command output:

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.

[milestone Milestone 6 ] Add examples/divergence_demo.py: load two pre-recorded dumps, invoke `replay_d...

1 participant