Skip to content

Fix module docstring extraction when a leading comment precedes it - #3315

Open
ayushcodes10 wants to merge 1 commit into
Graphify-Labs:v8from
ayushcodes10:fix-3312-docstring-leading-comment
Open

Fix module docstring extraction when a leading comment precedes it#3315
ayushcodes10 wants to merge 1 commit into
Graphify-Labs:v8from
ayushcodes10:fix-3312-docstring-leading-comment

Conversation

@ayushcodes10

@ayushcodes10 ayushcodes10 commented Sep 3, 2026

Copy link
Copy Markdown

_get_docstring's loop checked only the FIRST child of a module/class/
function body for a docstring expression_statement, then unconditionally
broke -- so a leading comment (the #! shebang essentially every
executable Python script starts with, a coding declaration, a license
header) sat in that first-child slot instead of the docstring, and the
real docstring one position later was never seen. No warning, no
partial-extraction report, exit 0 -- module rationale for an entire
class of file (any shebang-led or comment-led script) was silently
absent from the graph.

Comments are not statements: a docstring must be the first STATEMENT
in the body, so the fix skips leading comment children before
checking whether the first real statement is a docstring. The same
shared helper also serves class/function bodies, so a comment as the
first line of a function/class body is fixed too, not just the
module-level case reported in the issue.

Verified against the exact 3-file repro from the issue (before/after),
plus additional cases I traced through myself: a multi-line header
(shebang + coding declaration + license, 3 stacked comments), a
function/class docstring behind a leading in-body comment, a file with
a leading comment and genuinely no docstring (must not fabricate one),
and a string literal that is NOT the first statement (must still be
correctly rejected, not accidentally picked up by the new skip logic).

6 new tests in tests/test_rationale.py covering all of the above.

Full suite: 5117 passed, 0 failed. ruff and skillgen --check clean.

Fixes #3312.

_get_docstring's loop checked only the FIRST child of a module/class/
function body for a docstring expression_statement, then unconditionally
broke -- so a leading comment (the #! shebang essentially every
executable Python script starts with, a coding declaration, a license
header) sat in that first-child slot instead of the docstring, and the
real docstring one position later was never seen. No warning, no
partial-extraction report, exit 0 -- module rationale for an entire
class of file (any shebang-led or comment-led script) was silently
absent from the graph.

Comments are not statements: a docstring must be the first STATEMENT
in the body, so the fix skips leading `comment` children before
checking whether the first real statement is a docstring. The same
shared helper also serves class/function bodies, so a comment as the
first line of a function/class body is fixed too, not just the
module-level case.

Fixes Graphify-Labs#3312.

@graphify-labs graphify-labs Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Graphify reviewed this change.

Worth a look — the grounded gate found no coupling regressions or blocking issues, but 1 advisory finding(s) below merit a look before merge.

Formal verification. No changes could be formally verified in this run.


Graphify review — findings

Fixes Python module/class/function docstring extraction to skip leading comment nodes (shebangs, coding declarations, license headers) instead of stopping at the first one, so a docstring behind any comment prefix is found rather than silently dropped (#3312). Preserves the first-statement rule: a bare string that isn't the first real statement, or a body with no docstring at all, still yields no rationale node.

Worth a look

  • Leading-comment skip lets a non-first-statement string be misread as a docstringgraphify/extract.py:1223 · Escalate · medium
    • agreed by 2 of 2 members but NOT verified (no proof, no reproducing execution) — consensus is not a verdict; needs human review
Analysis details — impact, health, verification

Impact & health

Graphify review

Impact — 1776 functions depend on the 280 functions this change touches.

Health — this change adds coupling hotspots:

  • new: extract() — 520 callers, 43 callees
  • new: _rebuild_code() — 113 callers, 50 callees
  • new: extract_js() — 85 callers, 4 callees
  • new: extract_xaml() — 19 callers, 17 callees
  • new: dispatch_command() — 2 callers, 123 callees
  • new: _get_extractor() — 26 callers, 6 callees
  • new: run_pipeline() — 8 callers, 13 callees
  • new: collect_files() — 17 callers, 6 callees
  • …and 24 more — each is listed as a finding

Verification — 1776 functions in the blast radius were not formally verified this run (proofs are advisory here).

Gate & verification

graphify gate

PASS — objectively clean (no health regressions, tests not run — proofs not run this pass (advisory)). Grounded, not self-assessed.

Advisory (not blocking):

  • verification_scope: 1611 function(s) in the blast radius were not formally verified this run

Formal verification

Could not verify: Could not verify \_extract\_python\_rationale.

The verifier did not have enough to check \_extract\_python\_rationale, so it is saying so rather than guessing. No false assurance is the whole point.

Guarantee: No guarantee either way, this is an honest abstention, not a pass.

Note: Reason: parameter `path` is annotated `Path` — outside the synthesizable primitive/collection set

· 32 more finding(s) on lines outside this diff (see the check run).

@ayushcodes10

Copy link
Copy Markdown
Author

Correction: my earlier note about 7 pre-existing test failures was wrong. Those were caused by a stale pip-installed graphifyy 0.9.38 on my machine shadowing this checkout for every subprocess-based test (subprocess.run([sys.executable, "-m", "graphify"], ...) resolves via site-packages, not the local repo, unless installed editable). git stash correctly showed the failures were unaffected by my diff, but I concluded "pre-existing on v8" without checking why they failed — the real cause was my environment, not this codebase. Reinstalled editable (pip install -e .) and the full suite is clean: 5117 passed, 0 failed. Updated the PR description to match.

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.

Module docstring never extracted when a comment precedes it (shebang-led files lose all module rationale)

1 participant