feat(markdown): add CommentType.markdown backed by tree-sitter-markdown - #95
feat(markdown): add CommentType.markdown backed by tree-sitter-markdown#95HartmannNico wants to merge 2 commits into
Conversation
|
Nice, I will check this out. We are building the features in codelinks in sync with ubCode, so bear with me a little while I do that :) |
|
I approved the CI, it fails. |
|
pushed tests and doc as well. used pr #92 bash as template. |
Add support for Markdown HTML-comment traceability markers: Changes: - source_discover/config.py: add 'markdown' to COMMENT_FILETYPE (.md/.markdown) and CommentType.markdown enum value - analyse/utils.py: add MARKDOWN_QUERY '(html_block) @comment', wire tree_sitter_markdown in init_tree_sitter(); no SCOPE_NODE_TYPES entry (markdown has no function/class scopes; oneline-only mode) - pyproject.toml: add tree-sitter-markdown>=0.5.1 dependency Note: callers must set end_sequence=' -->' (not the default newline) because the html_block node text includes the full '<!-- ... -->' delimiters. Closes #NNN. Template: PR-82 (TypeScript).
…attern - Fix test_source_discover + test_src_trace: add 'markdown' to expected comment_type validation error message list - Add FE_MARKDOWN feature need to docs/source/components/features.rst (fixes docs build warnings for IMPL_MD_1/IMPL_MD_3/IMPL_LANG_1) - Add default_oneliner_markdown fixture to tests/data/extraction/oneline.yaml with end_sequence: " -->" config - Generate snapshot for the markdown extraction fixture - Add init_markdown_tree_sitter fixture + test_extract_comments_markdown + test_init_tree_sitter_markdown to tests/test_analyse_utils.py - Add 'markdown' entry to LANG_MAP in tests/test_extraction_fixtures.py - Update docs: configuration.rst (supported values + table row), analyse.rst (language support list), change_log.rst (Unreleased entry)
768670f to
09585c9
Compare
| # @Markdown HTML-comment query for tree-sitter, IMPL_MD_3, impl, [FE_MARKDOWN] | ||
| # Captures block-level HTML nodes (<!-- … -->) as @comment. Inline HTML comments | ||
| # inside paragraphs are not captured — only standalone html_block elements. | ||
| MARKDOWN_QUERY = """(html_block) @comment""" |
There was a problem hiding this comment.
(html_block) captures every block-level HTML element, not just HTML comments.
html_block is the CommonMark HTML block node: <div>, <details>, <table>, <script>, <br/> — all of them are handed to the extractor as "comments". Verified against tree-sitter-markdown==0.5.1:
<details>
<summary>@need-ids: FAKE_1</summary>
body
</details>get_need_id_refs defaults to True in SourceAnalyseConfig (and the Sphinx path only force-sets get_oneline_needs), so the above yields a need-ID reference with id FAKE_1</summary> — a phantom link with HTML markup baked into the ID, which is exactly FAULT_MARKDOWN_2. Same for <script>var x = 1; // @need-ids: REQ_9</script> → ref REQ_9.
A #match? predicate fixes it (verified working on the pinned tree-sitter~=0.25.1):
| MARKDOWN_QUERY = """(html_block) @comment""" | |
| MARKDOWN_QUERY = r"""((html_block) @comment (#match? @comment "^[ \t]*<!--"))""" |
| # markers. tree-sitter-markdown captures them as `html_block` nodes. | ||
| # NOTE: because the node text includes the `<!-- … -->` delimiters, callers | ||
| # must set `end_sequence: " -->"` (not the default `"\n"`) in their | ||
| # oneline_comment_style config to prevent `-->` from leaking into parsed fields. |
There was a problem hiding this comment.
Mandating end_sequence: " -->" silently opts Markdown out of the #88 prose anchor, so HTML comments containing prose hallucinate needs.
oneline_parser only anchors a marker to the start of the comment when end_sequence == UNIX_NEWLINE:
if oneline_config.end_sequence == UNIX_NEWLINE and any(
char.isalnum() for char in oneline[:start_idx]
):
return NoneWith " -->" that guard never runs, so every @ anywhere inside an HTML comment is parsed. Verified end-to-end:
<!-- reviewers: @alice, @bob, please check -->→ need{title: "alice", id: "@bob", type: "please check"}— a bogus ID that raisesInvalidNeedExceptionin Sphinx-Needs (the exact failure the 1.4.0 "Anchor newline-terminated one-line markers" fix was written for).<!-- @format -->(the Prettier pragma, common in Markdown) →too_few_fieldswarning, which failssphinx-build -nW.
Markdown files are the most prose-heavy input this project accepts and .md discovery sweeps every README.md/CHANGELOG.md under src_dir, so this is a high-volume false-positive source. Consider extending the anchor check to treat a delimiter-shaped end_sequence as line-anchored too, or strip <!--/--> in the Markdown path so the default newline end_sequence keeps working.
| default_oneliner_markdown: | ||
| lang: markdown | ||
| config: | ||
| end_sequence: " -->" |
There was a problem hiding this comment.
end_sequence: " -->" requires exactly one space before -->; ...--> silently extracts nothing.
<!-- @A Title, IMPL_A, impl, [R1]--> is valid HTML and a very common way to write the closing delimiter, but oneline.rfind(" -->") returns -1, so oneline_parser returns None: zero needs, zero warnings, no diagnostic. Verified against the real pipeline.
Same class of silent loss when two markers share a line: <!-- @A ... --> <!-- @B ... --> → rfind takes the last -->, both comments merge into one field list and you get a spurious too_many_fields warning instead of two needs.
Worth a fixture for the no-space form so the sharp edge is at least pinned by a test.
| config: | ||
| end_sequence: " -->" | ||
| source: | | ||
| <!-- @Md Title, IMPL_MD, impl, [REQ_MD] --> |
There was a problem hiding this comment.
Multi-line HTML comment blocks extract nothing under the mandated end_sequence: " -->".
The natural way to declare several Markdown markers is one HTML comment holding several lines:
<!--
@A Title, IMPL_A, impl, [R1]
@B Title, IMPL_B, impl, [R2]
-->extract_oneline_need parses the html_block text line by line; no inner line contains " -->" and the closing --> line contains no @, so the result is 0 needs and 0 warnings. With the default end_sequence: "\n" the same input yields both needs correctly (verified).
So the two Markdown comment forms are mutually exclusive per project config — one-line markers need " -->", multi-line blocks need "\n" — and picking the documented one makes the other fail silently. This is the real argument for handling the <!-- … --> delimiters inside the Markdown path instead of pushing end_sequence onto callers.
| # NOTE: because the node text includes the `<!-- … -->` delimiters, callers | ||
| # must set `end_sequence: " -->"` (not the default `"\n"`) in their | ||
| # oneline_comment_style config to prevent `-->` from leaking into parsed fields. | ||
| "markdown": ["md", "markdown"], |
There was a problem hiding this comment.
Nothing enforces the end_sequence requirement, so the default config silently corrupts need fields.
A user who configures only comment_type = "markdown" keeps end_sequence = "\n", and generate_project_configs force-enables get_oneline_needs. Verified results:
<!-- @Md Title, IMPL_MD, impl -->→ need withtype == "impl -->"— silently corrupted, no warning.<!-- @Md Title, IMPL_MD, impl, [REQ_MD] -->→not_start_or_end_with_square_bracketswarning, marker dropped.
A comment in COMMENT_FILETYPE and a .. note:: in the docs are the only things standing between a user and corrupted traceability data. This deserves enforcement in code rather than prose — e.g. a check in SourceAnalyseConfig.check_fields_configuration() that errors when comment_type is CommentType.markdown and oneline_comment_style.end_sequence == UNIX_NEWLINE, or a per-language default end_sequence.
| # oneline markers in Markdown are always standalone html_block nodes; | ||
| # scope association (find_enclosing_scope / find_next_scope) is never | ||
| # invoked when get_oneline_needs=True and get_need_id_refs=False. | ||
| # CommentType.markdown is intentionally absent from this table. |
There was a problem hiding this comment.
This justification is factually wrong: scope association is invoked for Markdown, on every comment, regardless of those flags.
SourceAnalyse.extract_marked_content calls it unconditionally before looking at either flag (analyse.py):
tagged_scope = utils.find_associated_scope(
src_comment.node, self.analyse_config.comment_type
)
if self.analyse_config.get_need_id_refs:
...
if self.analyse_config.get_oneline_needs:
...Because CommentType.markdown is absent from this table, find_next_scope/find_enclosing_scope silently fall back to the C++ scope set:
scope_types = SCOPE_NODE_TYPES.get(comment_type, SCOPE_NODE_TYPES[CommentType.cpp])so every html_block is walked to the end of the document looking for function_definition/class_definition. It returns None only because the Markdown grammar happens to have no node with those names — correct by luck, not by design, and O(blocks²) on a Markdown-heavy tree. An explicit CommentType.markdown: set() entry (or short-circuiting find_associated_scope for Markdown, as it already does for YAML/JSONC) would be both honest and cheaper.
| ``comment_type = "markdown"``. Because tree-sitter-markdown exposes | ||
| standalone HTML comments as ``html_block`` nodes, only block-level | ||
| ``<!-- … -->`` markers are captured; inline HTML comments inside paragraphs | ||
| are not. |
There was a problem hiding this comment.
This claim is inaccurate and understates the false-positive surface.
"Because tree-sitter-markdown exposes standalone HTML comments as html_block nodes, only block-level <!-- … --> markers are captured" — html_block is not comment-specific. <div>, <details>, <table>, <script> and <br/> blocks are all captured and scanned for markers (verified). What's true is the narrower statement that only block-level HTML is captured; inline HTML is not.
Either fix the query (see the comment on MARKDOWN_QUERY) and keep this sentence, or state plainly that all block-level HTML is scanned so users understand why <details> sections can produce phantom need IDs.
| def test_extract_comments_markdown(code, expected_count, init_markdown_tree_sitter): | ||
| parser, query = init_markdown_tree_sitter | ||
| comments = utils.extract_comments(code, parser, query) or [] | ||
| assert len(comments) == expected_count |
There was a problem hiding this comment.
The added tests cover only the happy path; every sharp edge of this feature is untested.
AGENTS.md: "Test coverage: Write tests for all new functionality and bug fixes." The four behaviours most likely to bite users all pass through untested:
- a non-comment
html_block(<div>,<details>,<script>) being captured as a comment; [R1]-->(no space before the delimiter) silently extracting nothing;- a multi-line
<!-- … -->block extracting nothing under the documentedend_sequence; - the default
end_sequenceleaking" -->"into astrfield.
Cases 2–4 are one-line additions to tests/data/extraction/oneline.yaml; case 1 fits here as an expected_count row.
| b"<!-- @Md Title, IMPL_MD, impl, [REQ_MD] -->\n", | ||
| ], | ||
| ) | ||
| def test_init_tree_sitter_markdown(code): |
There was a problem hiding this comment.
Nits in the new tests.
@pytest.mark.parametrize("code", [ ...one value... ])over a single input adds a parametrize id for no benefit — inline the bytes literal.comments = utils.extract_comments(...) or []intest_extract_comments_markdownmasks the return contract:extract_commentsreturnslist | None, and theexpected_count == 0case passes whether it returnsNoneor[]. Assert the shape you expect instead of coercing it.init_markdown_tree_sitterre-implementsutils.init_tree_sitter(CommentType.markdown)(same language, same query); the second test already proves the real factory works, so the fixture only adds a second construction path that can drift.
| New and Improved | ||
| ................ | ||
|
|
||
| - ✨ Added Markdown language support for the ``analyse`` module. |
There was a problem hiding this comment.
Commit and PR titles don't follow the documented format.
AGENTS.md, Commit Message Format: <EMOJI> <KEYWORD>: Summarize in 72 chars or less (#<PR>) with keywords ✨ NEW: / 🐛 FIX: / 🧪 TEST: …, and PR Title and Description Format: "Use the same as for the commit message format, but for the title you can omit the KEYWORD and only use EMOJI".
This branch uses Conventional Commits instead:
1c0402c feat(markdown): add CommentType.markdown backed by tree-sitter-markdown09585c9 test(markdown): add tests, fixtures, docs following PR #92 pattern- PR title:
feat(markdown): add CommentType.markdown backed by tree-sitter-markdown
Also, 1c0402c's body still carries the unfilled placeholder Closes #NNN. (the PR description says Closes #94).
| Key capabilities: | ||
|
|
||
| * HTML-comment (``<!-- … -->``) detection via tree-sitter | ||
| * Auto-discovery of ``.md`` and ``.markdown`` files |
There was a problem hiding this comment.
Auto-discovering .md makes source-page names collide with real documents.
generate_code_page derives the generated page name by stripping the suffix:
pagename = str((file_path.relative_to(app.outdir)).with_suffix(""))Until now no discoverable extension was also a Sphinx source extension. With .md discoverable, tracing README.md yields pagename README; in a MyST project (or any project with .md documents), html-collect-pages runs after the normal pages are written, so the source-tracing page silently overwrites the real document's HTML. Worth either documenting "don't point src_dir at your doc sources" or namespacing the generated pagename.
|
|
||
| parsed_language = Language(tree_sitter_bash.language()) | ||
| query = Query(parsed_language, BASH_QUERY) | ||
| elif comment_type == CommentType.markdown: |
There was a problem hiding this comment.
Ninth branch on a chain that a table would replace.
Adding a language now means touching five places: COMMENT_FILETYPE, CommentType, SCOPE_NODE_TYPES, a *_QUERY constant, and this if/elif chain — and the chain is at 10 branches against ruff's PLR0912 limit of 12, so the next two languages break it.
A single registry keeps the per-language knowledge in one row and makes "no scope types" an explicit value rather than a missing key:
LANGUAGE_REGISTRY = {
CommentType.markdown: ("tree_sitter_markdown", MARKDOWN_QUERY, frozenset()),
...
}with init_tree_sitter doing one importlib.import_module + Query(...) lookup. Not required for this PR, but this is the third language added on the same chain.
Closes #94
Summary
Adds
comment_type: markdownbacked bytree-sitter-markdown, enabling<!-- @needs … -->HTML-comment markers in Markdown files to be ingested as traceability nodes by sphinx-codelinks.Primary use case: AI agent and skill definition files (
.claude/agents/*.md,.github/prompts/*.md) that are the actual implementation artefacts of agentic systems and need to participate in a requirements → architecture → design → implementation traceability chain.Changes (3 files, ~24 lines)
source_discover/config.py"markdown": ["md", "markdown"]toCOMMENT_FILETYPEmarkdown = "markdown"toCommentTypeenumanalyse/utils.pyMARKDOWN_QUERY = """(html_block) @comment"""— tree-sitter-markdown maps standalone<!-- … -->blocks tohtml_blocknodeselif comment_type == CommentType.markdown:branch ininit_tree_sitter()importingtree_sitter_markdownSCOPE_NODE_TYPESentry: Markdown has no function/class scopes; oneline-only mode never invokes scope associationpyproject.tomltree-sitter-markdown>=0.5.1Caller note
Because
html_blocknode text includes the full<!-- … -->delimiters, callers must configureend_sequence: " -->"(not the default"\n") in theironeline_comment_style. This is the only behavioural difference from all othercomment_typeentries.Example conf:
Relation to other PRs
Analogous to PR #92 (bash). PR #82 (TypeScript) was the structural template.
This PR is independent of PR #92 — it is based directly on
mainand does not require bash support to be merged first.