Claude/project review ynoju5 by tommcd · Pull Request #2 · tommcd/mdtk

tommcd · 2026-07-19T18:14:46Z

No description provided.

- Add MIT LICENSE file (pyproject declared MIT but the file was missing) - Switch to SPDX license expression with license-files (PEP 639) - Add authors, keywords, and trove classifiers for PyPI - Raise requires-python to >=3.10 (3.8/3.9 are EOL) and target py310-py314 - Export convert_bookmarks, BookmarkError, and __version__ from mdtk - Single-source dev dependencies in a [dev] extra; tox now installs it - Modernize setuptools_scm config (version_file, setuptools>=77) Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01HhapuWM1PDW5VfYP72AXwc

- A folder with no <dl> of its own now raises an error instead of silently exporting the next folder's bookmarks (find_next crossed folder boundaries) - Preserve titles containing inline markup via get_text() instead of .string, which returned None and produced "Untitled" - Escape square brackets in titles and percent-encode parentheses, spaces, and angle brackets in URLs so generated markdown links parse - Stop re-wrapping BookmarkError in a catch-all handler; chain causes with "raise ... from" and narrow the caught exception types - Report available folder names when the requested folder is missing, and detect non-export input (e.g. Chrome's JSON bookmarks file) - Return the number of bookmarks written; CLI now prints a summary, making empty results visible - Add --version flag; drop the catch-all handler in main() so unexpected errors surface with a traceback - Expand test suite from 2 to 16 tests covering all of the above Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01HhapuWM1PDW5VfYP72AXwc

- Document the CLI (export steps, --folder, --version, example output), behavior notes, and the Python API - the README previously never mentioned the mdtk-bookmarks command - Use absolute URLs so links also work on PyPI - docs/index.md now includes README.md via pymdownx.snippets, removing the drift between the two - Remove Setup.md, a pasted setup chat transcript Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01HhapuWM1PDW5VfYP72AXwc

- Collapse CI to a single matrix job (3.10-3.14); the tox job duplicated every environment and carried leftover debug output and a hardcoded PYTHONPATH - Install dependencies from the [dev] extra instead of listing them in the workflow; fetch full history so setuptools_scm computes real versions - Publish workflow now runs lint+tests as a gate, builds with twine check, and publishes via OIDC trusted publishing (pypi environment) instead of a long-lived API token Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01HhapuWM1PDW5VfYP72AXwc

gemini-code-assist

Code Review

This pull request refactors the mdtk package to target Python 3.10+, modernizes the build configuration in pyproject.toml, and significantly improves the robustness of the Chrome bookmarks-to-markdown converter. Key enhancements include helper functions in bookmarks.py for text cleaning, escaping special characters, and robust HTML parsing, alongside a comprehensive test suite in tests/test_bookmarks.py. The review feedback recommends percent-encoding square brackets ([ and ]) in URLs to prevent breaking markdown link targets, and updating the test suite to verify this encoding.

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.

gemini-code-assist · 2026-07-19T18:16:15Z

+    for char, quoted in (
+        (" ", "%20"),
+        ("(", "%28"),
+        (")", "%29"),
+        ("<", "%3C"),
+        (">", "%3E"),
+    ):


URLs containing square brackets [ and ] (e.g., in query parameters like ?tags[]=python) can break markdown link parsing because parsers may confuse them with link syntax. It is safer to percent-encode them as %5B and %5D respectively.

Suggested change

for char, quoted in (

(" ", "%20"),

("(", "%28"),

(")", "%29"),

("<", "%3C"),

(">", "%3E"),

):

for char, quoted in (

(" ", "%20"),

("[", "%5B"),

("]", "%5D"),

("(", "%28"),

(")", "%29"),

("<", "%3C"),

(">", "%3E"),

):

Adopted in 531a279, with one deviation: brackets are encoded only in the path, query, and fragment. Encoding them in the authority as suggested would corrupt IPv6 literal hosts (http://[2001:db8::1]:8080/ → http://%5B2001:db8::1%5D:8080/), where the brackets are structural per RFC 3986. Test added covering both the ?tags[]=python case and IPv6 preservation.

Generated by Claude Code

gemini-code-assist · 2026-07-19T18:16:16Z

+def test_markdown_special_characters_are_escaped(tmp_path):
+    """Brackets in titles and parentheses in URLs must not break the link."""
+    src = write_export(
+        tmp_path,
+        """<DL><p>
+    <DT><H3>EXPORT_FOLDER</H3>
+    <DL><p>
+        <DT><A HREF="https://en.wikipedia.org/wiki/Bracket_(mathematics)">
+        Bracket (mathematics) [Wikipedia]</A>
+    </DL><p>
+</DL><p>""",
+    )
+    out = tmp_path / "output.md"
+
+    convert_bookmarks(src, out)
+
+    assert out.read_text(encoding="utf-8") == (
+        r"- [Bracket (mathematics) \[Wikipedia\]]"
+        "(https://en.wikipedia.org/wiki/Bracket_%28mathematics%29)\n"
+    )


Update the test to verify that square brackets in URLs are also correctly percent-encoded to prevent breaking markdown link targets.

Suggested change

def test_markdown_special_characters_are_escaped(tmp_path):

"""Brackets in titles and parentheses in URLs must not break the link."""

src = write_export(

tmp_path,

"""<DL><p>

<DT><H3>EXPORT_FOLDER</H3>

<DL><p>

<DT><A HREF="https://en.wikipedia.org/wiki/Bracket_(mathematics)">

Bracket (mathematics) [Wikipedia]</A>

</DL><p>

</DL><p>""",

)

out = tmp_path / "output.md"

convert_bookmarks(src, out)

assert out.read_text(encoding="utf-8") == (

r"- [Bracket (mathematics) \[Wikipedia\]]"

"(https://en.wikipedia.org/wiki/Bracket_%28mathematics%29)\n"

)

def test_markdown_special_characters_are_escaped(tmp_path):

"""Brackets in titles and parentheses/brackets in URLs must not break the link."""

src = write_export(

tmp_path,

"""<DL><p>

<DT><H3>EXPORT_FOLDER</H3>

<DL><p>

<DT><A HREF="https://en.wikipedia.org/wiki/Bracket_(mathematics)?[test]">

Bracket (mathematics) [Wikipedia]</A>

</DL><p>

</DL><p>""",

)

out = tmp_path / "output.md"

convert_bookmarks(src, out)

assert out.read_text(encoding="utf-8") == (

r"- [Bracket (mathematics) \\[Wikipedia\\]]"

"(https://en.wikipedia.org/wiki/Bracket_%28mathematics%29?%5Btest%5D)\\n"

)

Addresses review feedback on PR #2: bracketed query strings such as ?tags[]=python can confuse some markdown parsers. Unlike the suggested blanket replacement, the authority is left untouched because brackets there delimit IPv6 literal hosts (e.g. http://[2001:db8::1]:8080/), which encoding would corrupt. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01HhapuWM1PDW5VfYP72AXwc

Copilot

Pull request overview

This PR matures mdtk into a more complete “Chrome bookmarks export → Markdown” tool by strengthening the parsing/escaping logic, adding CLI/version behavior, expanding tests, and updating packaging/CI/docs to match the new API/UX.

Changes:

Reworked bookmark parsing to handle realistic Chrome exports (folder discovery, nested folder flattening, escaping) and return a bookmark count.
Expanded pytest coverage substantially, including CLI behaviors and edge cases.
Updated packaging/metadata, CI + release workflows, and MkDocs/README-driven documentation.

Reviewed changes

Copilot reviewed 11 out of 11 changed files in this pull request and generated 2 comments.

Show a summary per file

File	Description
tests/test_bookmarks.py	Replaces minimal tests with broad coverage incl. edge cases and CLI expectations.
src/mdtk/bookmarks.py	Implements robust folder/list detection, escaping, count return, and CLI `--version` support.
src/mdtk/init.py	Exposes public API (`convert_bookmarks`, `BookmarkError`) and package version.
Setup.md	Removes setup scaffolding document from repo.
README.md	Adds detailed usage docs, API example, dev commands, and badges.
pyproject.toml	Updates Python requirement/metadata, dev extras, ruff/tox config, and setuptools_scm settings.
mkdocs.yml	Enables snippets extension for README-driven docs.
LICENSE	Adds MIT license file.
docs/index.md	Switches docs homepage to include README via snippets.
.github/workflows/publish.yml	Adds a test gate before publishing and uses trusted publishing (OIDC).
.github/workflows/ci.yml	Simplifies CI to a single matrix job with lint + tests.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

tommcd · 2026-07-19T19:52:27Z

 [tool.setuptools_scm]
-write_to = "src/mdtk/_version.py"
+version_file = "src/mdtk/_version.py"


version_file is the current name for this option in setuptools_scm ≥8 (2023), which the build-system pins (setuptools_scm[toml]>=8); write_to is the deprecated spelling. Verified empirically on this branch: python -m build produces a wheel containing the generated mdtk/_version.py, and the editable install writes src/mdtk/_version.py. Keeping version_file.

Generated by Claude Code

tommcd · 2026-07-19T19:52:33Z

- [PyPI Package](https://pypi.org/project/mdtk/)
- [Issue Tracker](https://github.com/tommcd/mdtk/issues)
- [Changelog](https://github.com/tommcd/mdtk/releases)
+--8<-- "README.md"


Partially right — snippets resolve relative to the current working directory (base_path default ['.']), not docs_dir, so the include worked when mkdocs runs from the repo root (verified: this branch's docs build passes and the rendered page contains the README content). But that was CWD-fragile: building from any other directory failed. Fixed in 2d08664 by anchoring base_path: !relative $config_dir; verified the build now succeeds from both the repo root and a foreign CWD.

Generated by Claude Code

pymdownx.snippets defaults to resolving paths against the current working directory, so the docs build only worked when mkdocs ran from the repo root. Anchor base_path to the mkdocs.yml location so the build is CWD-independent. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01HhapuWM1PDW5VfYP72AXwc

claude added 4 commits July 19, 2026 15:24

Copilot AI review requested due to automatic review settings July 19, 2026 18:14

gemini-code-assist Bot reviewed Jul 19, 2026

View reviewed changes

Copilot started reviewing on behalf of tommcd July 19, 2026 18:39 View session

Copilot AI reviewed Jul 19, 2026

View reviewed changes

tommcd merged commit 264fd6b into main Jul 19, 2026
5 checks passed

tommcd deleted the claude/project-review-ynoju5 branch July 19, 2026 20:45

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Claude/project review ynoju5#2

Claude/project review ynoju5#2
tommcd merged 6 commits into
mainfrom
claude/project-review-ynoju5

tommcd commented Jul 19, 2026

Uh oh!

gemini-code-assist Bot left a comment

Uh oh!

gemini-code-assist Bot Jul 19, 2026

Uh oh!

tommcd Jul 19, 2026

Uh oh!

gemini-code-assist Bot Jul 19, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

tommcd Jul 19, 2026

Uh oh!

tommcd Jul 19, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Conversation

tommcd commented Jul 19, 2026

Uh oh!

gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

Code Review

Uh oh!

gemini-code-assist Bot Jul 19, 2026

Choose a reason for hiding this comment

Uh oh!

tommcd Jul 19, 2026

Choose a reason for hiding this comment

Uh oh!

gemini-code-assist Bot Jul 19, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

tommcd Jul 19, 2026

Choose a reason for hiding this comment

Uh oh!

tommcd Jul 19, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants