Skip to content

tracer: keep the whole block body, and say why capture failed - #724

Merged
JadenFiotto-Kaufman merged 1 commit into
0.8from
fix/tracer
Sep 8, 2026
Merged

JadenFiotto-Kaufman merged 1 commit into
0.8from
fix/tracer

Conversation

@JadenFiotto-Kaufman

@JadenFiotto-Kaufman JadenFiotto-Kaufman commented Sep 8, 2026

Copy link
Copy Markdown
Member

Two findings from the agent stress sweep, both in the with-block capture path
(items 14.2 and 22 in the review notes).

A column-0 comment truncated the trace body

Tracer._parse_block ended its slice at any non-blank line indented back to the
header's column. A comment is non-blank and Python ignores its indentation, so
commented-out code at column 0 — where every editor leaves it — cut the body
short. The truncated slice still parsed as a valid ast.With, so the whole-file
fallback never fired and the statements after the comment silently never ran.

Blanks and comments no longer end the slice. The docstring's "It never returns a
wrong node" was exactly wrong for this case and now says which bound is unsafe
and why.

Before / after on the repro
(tasks/trace-body-capture-and-save/skills/repro_7_col0_comment_truncates_body.py):
NameError: name 'o' is not definedafter comment ran / o = 2.0.

WithBlockNotFoundError carried no message

raise WithBlockNotFoundError() gave the bare class name for at least five
distinct causes — a trace typed into stdin, python -c, an exec'd string not
in linecache, a %%time cell magic, an edit that shifted line numbers — and
the user could not tell them apart. _not_found_message now branches on what the
raise site knows:

  • no source recovered at all (stdin, an exec'd string not in linecache) —
    say where a trace body has to live;
  • a real file with no with at that line — name the file, line and the text of
    that line, and say the source looks stale (nnsight reads each file once) or
    that the tracer was entered without a with;
  • otherwise — name file and line and explain that nnsight compiles the block's
    own body, so line numbers have to match (a %%time cell magic, generated
    source out of step with its linecache entry).

docs/errors/index.md now carries the real message text for all three.

Deliberately not fixed: the stale compiled-body cache itself (BLOCKS/SOURCES
never re-validated), per the review. The second branch only reports that the
source looks stale.

Docs

docs/errors/index.md, plus the two lines my change made false —
docs/errors/with-block-not-found.md and docs/errors/symptom-index.md both
asserted the exception carries no message — and the _parse_block bound and
_not_found_message sentences in docs/developing/tracing-pipeline.md.

Not in this PR

An earlier revision of this branch also restored the thread's trace function
around a block (skip_context clobbers it, __exit__ sets it to None, so a
debugger breakpoint set after a trace never fires and coverage reports every line
after the first block as missed). The maintainer has decided against that change,
and it has been removed in full: skip_context installs the bare no-op and
__exit__ keeps its unconditional sys.settrace(None), exactly as on 0.8.

Testing

PYTHONPATH=<worktree>/src throughout, so the worktree is what ran.

  • tests/test_tracing.py: 63 passed (56 before; 7 added). The two comment tests
    fail on origin/0.8 and pass here — verified against a scratch checkout of
    origin/0.8's src. The four _not_found_message tests are new-API and can
    only run here.
  • Full suite minus tests/vllm, tests/tp, tests/performance: 1008 passed,
    3 skipped, 1 xfailed. origin/0.8 on the same command: 1001 passed, same
    skips — the difference is exactly the 7 added tests.
  • Serialization round-trips re-checked specifically (12 tests across
    test_serialization.py, test_editing.py, test_diffusion.py,
    test_encoder.py): nothing here stores anything new on the Tracer, and
    __getstate__/__setstate__ are untouched relative to 0.8.
  • vLLM and tensor-parallel tests were not run: no vLLM in this environment and no
    multi-GPU. Neither item touches those paths.
  • Both repros named in the review behave as expected.

Note: pytest tests/test_serialization.py on its own dumps core partway through
in this environment on origin/0.8 too — pre-existing, unrelated, and it does
not happen in the full run.

🤖 Generated with Claude Code

A comment at column 0 inside a trace body truncated it: `_parse_block` ended the
slice at any non-blank line indented back to the header's column, and Python
ignores a comment's indentation. What was left still parsed as a `with`, so the
whole-file fallback never fired and the statements after the comment silently
never ran. Blanks and comments no longer end the slice, and the docstring no
longer claims the slice never returns a wrong node — this was the case where it
did.

`WithBlockNotFoundError` carried no message, and at least five unrelated causes
reach it. It now says which — no readable source, a file whose source has gone
stale, or source whose line numbers don't match the running code.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@JadenFiotto-Kaufman JadenFiotto-Kaufman changed the title tracer: keep the caller's trace function, the whole block body, and say why capture failed tracer: keep the whole block body, and say why capture failed Sep 8, 2026
@JadenFiotto-Kaufman
JadenFiotto-Kaufman merged commit 5f84df7 into 0.8 Sep 8, 2026
2 checks passed
@JadenFiotto-Kaufman
JadenFiotto-Kaufman deleted the fix/tracer branch September 8, 2026 19:12
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