fix: derive notebook cell boundaries from the AST, not a line prefix

[Jammy2211]

Overview

add_notebook_quotes decides what is a narrative docstring with a line-prefix
test, so a code string literal's closing delimiter — which usually sits at
column 0 — is mistaken for a docstring boundary. Every cell boundary after it is
inverted: the enclosing code cell becomes a hard SyntaxError, and the code that
follows is emitted as a markdown cell.

Confirmed live on origin/main (autohands/add_notebook_quotes.py:152). It is
latent, not shipping — the only workspace file with the shape,
autolens_workspace_test/gallery/gallery_build.py:42, sits outside scripts/
and iter_script_paths only walks scripts/, so it is never converted. Same
failure class as #211's opener bug, different trigger.

Splitting the converted reproducer the way ipynb-py-convert's py2nb does
(on the literal "\n\n# %%\n"):

cell	kind	result
1	code	SyntaxError: unterminated triple-quoted string literal
2	markdown	print(s) — real code rendered as narrative prose

Plan

• Derive narrative-docstring boundaries from the parsed source instead of a line
  prefix, so a string bound to a name can never be mistaken for a docstring.
• Route both segmentation sites in add_notebook_quotes.py through that one
  shared helper. navigator.py needs no change — it already delegates to
  add_notebook_quotes and reads the ''' delimiters back out, so it inherits
  the fix.
• Fail loudly on the two shapes the new segmentation cannot express — an
  unparseable source script, and a column-0 single-line docstring — rather than
  silently emitting a mangled cell (the fix: split a docstring that follows code into its own notebook cell #214 stray-# %% precedent).
• Add regression tests for the string-literal shape, including the exact
  reproducer and the real gallery_build.py shape.
• Prove the change is behaviour-preserving by regenerating the artifact-bearing
  workspaces and requiring a zero diff — no live script has the shape, so a
  correct fix must change nothing.

Detailed implementation plan

Work Classification

Library — source + tests in a single organ repo. Ships via ship_library.

Affected Repositories

• PyAutoHands (primary)

Branch Survey

Repository	Current Branch	Dirty?
./PyAutoHands	main	clean

worktree_list_claimed: nothing registered. worktree_check_conflict exit 0 —
no conflict. main is the only branch; no unregistered prior-session work.

Suggested branch: feature/notebook-quotes-string-literal
Worktree root: ~/Code/PyAutoLabs-wt/notebook-quotes-string-literal/

Implementation Steps

1. New helper _narrative_docstring_ranges(lines) in
   autohands/add_notebook_quotes.py, returning 0-based (start, end) line
   index pairs:

   • ast.parse("".join(lines)); walk module.body only.
   • Accept a node when it is ast.Expr wrapping ast.Constant of str, with
     col_offset == 0, and the source line at lineno - 1 begins with """ or
     '''. This reproduces the intended semantics of today's test (a column-0
     bare triple-quoted expression statement) while excluding s = """…""",
     indented function/class docstrings, and non-triple-quoted string statements.
   • SyntaxError → re-raise as ValueError quoting the offending line, so a
     broken source script fails the build rather than producing a broken notebook.
   • start == end (single-line docstring) → ValueError naming the line. Zero
     occurrences across the workspaces (see Reachability), so rejecting it is
     cheaper and louder than emitting a one-line markdown cell.

2. add_notebook_quotes (:151-176) — the actual fix. Iterate
   enumerate(lines); replace the line.startswith('"""') test at :152 with
   membership in the boundary set {start} ∪ {end} built from the helper.
   All downstream cell-marker logic (:153-176) is untouched.

3. strip_env_declarations (:51) — hardening, not a fix. Verified
   not to be a live defect: the false "opener" match does fire, but every
   non-__Env__ path emits the block verbatim (:81), so the misparse is
   absorbed. It would only bite if an __Env__ header sat between a literal's
   closer and the next column-0 delimiter — i.e. outside any docstring, which is
   not a real shape. Migrated anyway so there is one segmentation source rather
   than two prefix tests that can drift. close becomes the helper's end
   instead of the forward scan at :55-58; the __Env__ header scan and the
   merged/standalone/emptied branches (:61-83) are unchanged. The
   close == n unterminated branch becomes unreachable and is removed —
   unterminated is now a parse error.

4. Docstring — update the "single shared strip layer" paragraph (:30-35)
   to say the segmentation is AST-derived and that navigator.py inherits it
   through add_notebook_quotes.

5. tests/test_add_notebook_quotes.py — add: the reproducer below, asserting
   [markdown, code] with the literal intact and no cell source containing
   # %%; the real gallery_build.py shape (CSS = """ … column-0 """ …
   def); a s = '''…''' single-quote variant; and a syntactically broken
   source raising ValueError.

6. tests/test_strip_env_declarations.py — one case: an __Env__ block
   followed by a code string literal, pinning that the literal survives the strip
   untouched.

Reachability

Scanned all 1517 .py files across the four workspaces, their _test /
_developer siblings and the three HowTo repos:

• 0 files fail ast.parse → the AST route is safe everywhere.
• 0 column-0 single-line docstrings → the one shape AST treats differently
  from the current prefix test has no live occurrence.
• 1 column-0 triple-quote closer belonging to a code string:
  autolens_workspace_test/gallery/gallery_build.py:42 — outside scripts/,
  never converted.

Validation

• pytest PyAutoHands/tests/ green — in particular test_add_notebook_quotes.py,
  test_strip_env_declarations.py, test_check_navigator.py,
  test_generate_markdown.py (currently 26 passed on the first three).
• Regenerating the six artifact-bearing workspaces produces a zero diff. This
  is the real proof of the strip_env_declarations migration: a non-zero diff
  means the AST predicate disagrees with the old prefix test somewhere.
• navigator catalogues unchanged, for the same reason.

Trade-off — ast over tokenize.generate_tokens

Both give exact line spans. ast distinguishes "bare string expression
statement" from "string bound to a name" directly as a node type, where
tokenize requires reconstructing that from surrounding NEWLINE/NL/INDENT
context. Same information, less hand-rolled state. Both require parseable source.

Key Files

• autohands/add_notebook_quotes.py — the two segmentation sites and the new helper.
• autohands/navigator.py — no change; _docstring_blocks inherits the fix.
• tests/test_add_notebook_quotes.py, tests/test_strip_env_declarations.py — regressions.

Original Prompt

Click to expand starting prompt

add_notebook_quotes mistakes a code string literal's closing delimiter for a docstring

Type: bug
Target: hands
Repos:

• PyAutoHands
  Difficulty: small
  Autonomy: safe
  Priority: low

add_notebook_quotes decides what is a narrative docstring with a line-prefix
test (add_notebook_quotes.py):

if line.startswith('"""') or line.startswith("'''"):

A triple-quoted string literal assigned in code opens on a line that does
not start at column 0 (s = """), so the opener is invisible to the test —
but its closing delimiter usually sits at column 0, and that line does
match. It flips is_in_quotes, and every cell boundary after it is inverted.

Reproduced:

"""
__Intro__
"""

x = 1
s = """
literal
"""
print(s)

converts to a final code cell whose source is

x = 1
s = """
literal
# %%
'''
print(s)

— the literal's closing delimiter became a cell marker, and print(s) landed
inside a broken string. Same failure class as
#211's opener bug, different trigger.

Reachability

One occurrence workspace-wide, found by scanning every .py in the four
workspaces, their _test / _developer siblings, and the three HowTo repos for
a column-0 triple-quote-closing line belonging to a code string:

• autolens_workspace_test/gallery/gallery_build.py:42

gallery/ sits outside scripts/, and iter_script_paths only walks
scripts/, so this file is never converted — the bug is latent, not
shipping. That is why it was left out of #211 rather than fixed there.

Fix

Replace the line-prefix test with real tokenization. tokenize.generate_tokens
yields STRING tokens with exact start/end line numbers, which distinguishes
a module/narrative docstring (a bare STRING expression statement) from a string
bound to a name. strip_env_declarations and navigator.py share this same
tokenizer-by-line-prefix assumption (add_notebook_quotes.py docstring: "the
single shared strip layer"), so all three should move together or the catalogue
and the notebooks will disagree.

Cheaper interim option, if tokenizing is judged too invasive: raise on a column-0
triple-quote line that closes a string the scanner never saw opened. Loud beats a
silently mangled cell, and matches the stray-# %% guard added in
#214.

Validation

• The reproducer above yields [markdown, code] with the literal intact inside
  the code cell, and no cell source containing # %%.
• The existing tests/test_add_notebook_quotes.py suite stays green.
• Regenerating all six artifact-bearing workspaces produces a zero diff —
  no live script has the shape, so a correct fix must change nothing.
• navigator catalogues are unchanged for the same reason.

Notes

• Found 2026-07-30 while fixing the docstring-after-code split
  (fix: notebook generation mangles a docstring that follows code; drop the Finish. hack #211 / fix: split a docstring that follows code into its own notebook cell #214).

[Jammy2211]

Library PR Created

PR: #246 — feature/notebook-quotes-string-literal → main, labelled pending-release.

Confirmed live before any code was written. origin/main's
add_notebook_quotes.py is byte-identical to the planned-against copy
(md5 8a53800…); the only commit the checkout was behind was a release-board
docs auto-update. The reproducer converts exactly as reported.

Fix: cell boundaries now come from _narrative_docstring_ranges, which parses
the script and takes module-level bare string expression statements. ast
separates a docstring from a string bound to a name by node type.

Two corrections to the plan on the issue, both reported rather than quietly absorbed:

1. strip_env_declarations is not a live defect. The plan claimed it shared
   the bug. It doesn't — the false "opener" match fires, but every non-__Env__
   path emits the block verbatim, so the misparse is absorbed. It would only bite
   if an __Env__ header sat between a literal's closer and the next column-0
   delimiter, i.e. outside any docstring, which is not a real shape. It is
   migrated to the shared helper as hardening only.
2. A second latent defect was found and fixed in passing. The prefix test also
   missed an indented closing delimiter, so the block never closed and all
   following lines were swallowed into the markdown cell — the mirror of fix: notebook generation mangles a docstring that follows code; drop the Finish. hack #211's
   opener bug. Three files carry the shape, all in
   autocti_workspace_test/legacy/tvac/, all outside scripts/.

Validation

check	result
pytest PyAutoHands/tests/	349 passed (was 340)
Control test — new tests vs pre-fix module	8 of 9 fail, as they must
Zero-diff, function level — 693 converted scripts	0 differences, 0 new raises
Zero-diff, end-to-end through ipynb-py-convert — 60 real scripts	0 notebook JSON differences
Boundary-divergence audit — all 1517 workspace .py	18 divergences, all outside scripts/, all correct

The control test is the one that matters: the 8 behavioural regressions were run
against the original implementation and all 8 fail there. The 9th
(test_strip_leaves_a_code_string_literal_untouched) passes before and after —
correctly, since that function was never broken; it is a pin, not a regression.

Heart gate

stale / score 85, one reason: release validation stale: source moved since rehearsal (PyAutoGalaxy). No RED, no YELLOW. Unrelated to notebook conversion
and a release-validation concern rather than a feature-PR blocker; the human
acknowledged it at ship time. PR-open releases nothing — merge remains a
separate human act.

Workspace impact — (iii) none

No workspace script references add_notebook_quotes, strip_env_declarations or
the new helper; no related no_run.yaml entries. Generated notebooks are
byte-identical for every converted script, so no regeneration is required and no
workspace follow-up is needed.

/smoke_test was not run: it executes workspace scripts, and this change is
confined to the notebook generator, which no script execution touches. The
applicable equivalent — the end-to-end notebook comparison above — was run
instead.

[Jammy2211]

Shipped

Merged as 10a3178 via #246 (squash). CI on the PR head was
green on all three legs (pytest 3.12 / 3.13 / 3.14), merge state CLEAN.

Cell segmentation in add_notebook_quotes is now derived from the parsed source
(_narrative_docstring_ranges) rather than a line prefix, so a triple-quoted
string literal bound to a name can no longer be read as a docstring boundary.
The same change closes an indented closing delimiter — the mirror defect the
prefix test also carried — and raises loudly on an unparseable script or a
column-0 single-line docstring instead of emitting a broken cell.

No workspace follow-up: generated notebooks are byte-identical for every
converted script, verified at the function boundary (693 scripts) and end-to-end
through ipynb-py-convert (60 scripts).