fix: collapse SPDXMarkdown into single file - #5
Merged
Conversation
NOTE: We already use `ty` for type checking and `ruff` for linting. Ref: #4
Archive the completed `docs-best-practices` change after all tasks were implemented and verified. - All 5 tasks across 4 spec areas were marked done before archiving. - Completed tasks: Enhanced docs landing page, enhanced API reference, created usage guide (docs/guide.md), polished changelog, final verification (tests + docs-build). - Moves all docs-best-practices artifacts under `archive/2026-06-22-docs-best-practices/`. Ref: #4
Commit the three synced delta specs from archiving the docs-best-practices change: - docs-api-enhancement - docs-guide - docs-landing Ref: #4
Create the collapse-impl-module change to define the implementation plan for collapsing the two-file architecture into a single module. - Define scope of the refactor via proposal and design artifacts. - Establish a new spec for the `SPDXMarkdown` formatter behavior. - Outline tasks for implementing each step. Ref: #4
Mark all tasks as done after implementing the single-file collapse of the SPDXMarkdown formatter. - All verification tasks passed (ruff, tests, smoke test). - Plan and OpenSpec artifacts updated to reflect actual implementation. Ref: #4
Update proposal, design, and specs to reflect the actual implementation: Single-file architecture with `SPDXMarkdown` wrapper defined first, then `_SPDXMarkdownImpl(SPDXMarkdown, Markdown)` after commitizen imports. - Design: Document multiple-inheritance approach and __new__ override. - Proposal: Correct the circular-import description. - Spec: Update requirements to match actual architecture. Ref: #4
Add row and reference link for the new action plan. Ref: #4
The implementation now lives in spdx_markdown.py as `_SPDXMarkdownImpl(SPDXMarkdown, Markdown)`. Ref: #4
Merge _impl.py into spdx_markdown.py. Define SPDXMarkdown wrapper first (avoids circular import), then _SPDXMarkdownImpl(SPDXMarkdown, Markdown) after commitizen imports fix isinstance checks via multiple inheritance. - Override __new__ on impl class to prevent recursion. - Remove import sys as _sys (unused). - Suppress E402 with noqa (inherent to staggered imports). Ref: #4
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request collapses the formatter implementation from a two-file structure into a single spdx_markdown.py file, resolving an isinstance type-checking bug and removing the _impl.py file. It also removes basedpyright from the development dependencies. The reviewer feedback suggests a cleaner approach to solving the isinstance issue: using virtual subclassing via Python's abc module instead of multiple inheritance and __new__ overrides, which avoids fragile recursion workarounds.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
Use abc.ABCMeta virtual subclassing instead of multiple inheritance to satisfy isinstance checks, addressing Gemini code review feedback. - SPDXMarkdown inherits from abc.ABC for register() support - SPDXMarkdown.register(_SPDXMarkdownImpl) for virtual subclassing - _SPDXMarkdownImpl inherits only Markdown, no __new__ override. Ref: #4, PR #5
Update all OpenSpec artifacts to describe `abc.ABC` virtual subclassing instead of multiple inheritance with the `__new__` recursion workaround. - Delta spec: Add ADDED Requirements header for archive compatibility. - Design: Update decision section, rationale, and alternatives. - Proposal: Change multiple inheritance to abc.ABCMeta virtual subclassing. - Tasks: Clarify single-file implementation description. Ref: #4, PR #5
Update plan to reflect actual implementation using abc.ABC virtual subclassing instead of multiple inheritance with __new__ recursion workaround. - Resolution section: Describe SPDXMarkdown(abc.ABC) wrapper and _SPDXMarkdownImpl(Markdown) with register(). - Tasks and acceptance criteria updated accordingly. Ref: #4, PR #5
redtux
added a commit
that referenced
this pull request
Jun 23, 2026
Use abc.ABCMeta virtual subclassing instead of multiple inheritance to satisfy isinstance checks, addressing Gemini code review feedback. - SPDXMarkdown inherits from abc.ABC for register() support - SPDXMarkdown.register(_SPDXMarkdownImpl) for virtual subclassing - _SPDXMarkdownImpl inherits only Markdown, no __new__ override. Ref: #4, PR #5
redtux
added a commit
that referenced
this pull request
Jun 23, 2026
Update all OpenSpec artifacts to describe `abc.ABC` virtual subclassing instead of multiple inheritance with the `__new__` recursion workaround. - Delta spec: Add ADDED Requirements header for archive compatibility. - Design: Update decision section, rationale, and alternatives. - Proposal: Change multiple inheritance to abc.ABCMeta virtual subclassing. - Tasks: Clarify single-file implementation description. Ref: #4, PR #5
redtux
added a commit
that referenced
this pull request
Jun 23, 2026
Update plan to reflect actual implementation using abc.ABC virtual subclassing instead of multiple inheritance with __new__ recursion workaround. - Resolution section: Describe SPDXMarkdown(abc.ABC) wrapper and _SPDXMarkdownImpl(Markdown) with register(). - Tasks and acceptance criteria updated accordingly. Ref: #4, PR #5
8 tasks
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
1 participant
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.
Description
Collapse the two-file
SPDXMarkdownarchitecture into a single module. Theoriginal two-file split (
spdx_markdown.py+_impl.py) with a__new__-basedlazy wrapper was necessary to avoid a circular import during commitizen's
entry-point scanning, but introduced a latent bug:
isinstance(formatter, SPDXMarkdown)returnedFalsebecause_SPDXMarkdownImpldid not subclassSPDXMarkdown.This PR merges
_impl.pyintospdx_markdown.py, keeps the__new__pattern(circular import is real), and fixes
isinstancevia multiple inheritance:_SPDXMarkdownImpl(SPDXMarkdown, Markdown).Changes
src/.../spdx_markdown.py_SPDXMarkdownImpl(SPDXMarkdown, Markdown)with__new__overridesrc/.../_impl.pyspdx_markdown.pydocs/plans/10-collapse-impl.mddocs/plans/index.mdopenspec/changes/collapse-impl-module/Verification
uv run pytest -v(19/19)ruff check src/(E402 suppressed)uv build && unzip -l dist/*.whl | grep py.typedmake reuseCloses #4