🧪 Capture marker scope association in the declarative extraction fixtures - #100
Draft
ubmarco wants to merge 5 commits into
Draft
🧪 Capture marker scope association in the declarative extraction fixtures#100ubmarco wants to merge 5 commits into
ubmarco wants to merge 5 commits into
Conversation
find_associated_scope's result (tagged_scope) flowed into production output but was invisible to the declarative extraction-fixture harness, which normalized only needs/need_refs/marked_rst/warnings. Add a "scope" key to both need and need_ref entries: the associated node's type plus the first (stripped) line of its text, or null when there is none. Production still serialises the full node text; the harness only needs enough to prove the same declaration was selected, without dumping whole function bodies into expected JSON.
Recapture all existing declarative extraction-fixture snapshots for the new "scope" key added to need/need_ref entries. Every diff is additive: the "scope" key only. Reviewed each case's captured scope_type/scope_first_line by hand; all match the source's expected association (libclang-engine cases show null, since the analysis explicitly skips scope association for libclang comments).
Add tests/data/extraction/scope.yaml, pinning find_associated_scope's outcome across the languages with scope machinery: cpp, python, csharp, rust, go, bash, yaml. Each language gets a marker directly above a function/method (binds to it), a marker at the end of a function body (falls back to the enclosing function/method via find_enclosing_scope), and a marker with nothing scope-like around it (null). Python additionally covers a docstring marker (find_enclosing_scope path). YAML additionally covers its bespoke find_yaml_associated_structure path: an inline same-row comment, a leading comment bound to a following key-value pair, and one bound to a following sequence item. ts/tsx and jsonc are intentionally not covered here.
The declarative extraction snapshots only captured a reduced projection
of the real per-marker payload (id/title/type/links/metadata/line/scope
for needs, need_id/line/scope for refs), leaving filepath, remote_url,
the full source_map (columns and end positions), the MarkedContentType
discriminator, the need-ref marker, and the real tagged_scope text
completely untested.
Extend _normalize() to surface the fields every Metadata subclass
(OneLineNeed/NeedIdRefs/MarkedRst) actually carries: filepath (relative
to tmp_path, since production's absolute path differs per run/machine),
remote_url (deterministically null here — the harness forces
git_remote_url/git_commit_rev to None before run()), the full
source_map, a content_type discriminator (named to avoid colliding with
a need's own "type" field), and scope as {scope_type, scope_text} with
the node's full decoded text exactly as production serializes it
(replacing the truncated {scope_type, scope_first_line} shape). marker
is now included on need_ref entries. The existing needs decomposition
and the line/start_line/end_line keys are kept as-is.
Capturing full source_map fidelity surfaced two pre-existing production
issues, left unfixed here (out of scope): need_id_refs' extract_marker
computes start_column/end_column from the pre-strip() position but the
post-strip() length, so every need-ref's column span is shifted by the
stripped whitespace; and MarkedRst's source_map for multi-line RST
blocks collapses start/end to the same row and uses raw offsets into
the flattened multi-line comment text as "columns", which isn't a real
position for anything past the first line.
No fixture YAML changed; only the harness and the recaptured snapshots.
Update the extraction-fixture README's normalized-contract section to match the new snapshot shape: filepath, remote_url, source_map, content_type, and scope (scope_type + full scope_text) on every entry, plus marker on need_refs. Replace the old "volatile data is omitted" line with a per-field description and a trimmed exclusion list (only the tmp_path prefix and the raw comment/node objects are actually excluded now).
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #100 +/- ##
==========================================
+ Coverage 92.38% 92.40% +0.02%
==========================================
Files 43 43
Lines 3743 3754 +11
Branches 381 382 +1
==========================================
+ Hits 3458 3469 +11
Misses 172 172
Partials 113 113 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Marker scope association — which declaration a marker attaches to — is computed by
find_associated_scopeand flows into the output (analyse/models.pyserialises theassociated node's text;
needextend_write.pycarries it). But the shared declarativefixture harness normalised only
{needs, need_refs, marked_rst, warnings}and capturednothing scope-derived, so scope behaviour had no coverage in the fixture layer at all.
This PR closes that gap. It is deliberately scoped to the existing languages so it can
merge independently of the in-flight TypeScript work;
ts/tsxfixtures follow separately.What changed
_normalizein the fixture harness now records scope on everyneedsandneed_refsentry as
{"scope_type": <node kind>, "scope_first_line": <first line, stripped>}, ornullwhen there is no associated scope. Production serialisation is unchanged — it emitsthe associated node's entire text, which would put whole function bodies into expected
JSON. Type plus first line proves the same declaration was selected while staying legible.
scopekey.tests/data/extraction/scope.yamlwith 23 cases covering cpp, python, csharp,rust, go, bash and yaml: marker above a function, marker inside a function body (the
enclosing-scope fallback), no-scope, the python docstring path (which uses
find_enclosing_scoperather thanfind_next_scope), and yaml's bespoke structurefinder for inline-same-row, leading key/value and leading list-item association.
tests/data/extraction/README.mdcorrected — it documented the tagged scope as "omitted".jsoncis intentionally not covered: its association goes through a separate bespokefinder and is better pinned alongside a decision about data-format association generally.
Verification
tox -e py312-sphinx8-needs5: 345 → 368 passed / 1 skipped (+23, exactly the new cases)tox -e docs-clean: no new warningsFollow-up this surfaced
Every libclang-engine case captures
scope: null, because the preprocessor-aware path nullstagged_scopeoutright. That looks wrong rather than intentional: libclang decides onlywhether a marker is active, not which declaration it belongs to, so those markers should
carry the same scope information as the tree-sitter path. Being tracked separately — the
fixtures now make the gap visible instead of invisible.