Skip to content

fix(ci): replace yq with Python nav patching in publish-devnotes#548

Merged
andreatgretel merged 1 commit intomainfrom
andreatgretel/fix/publish-devnotes
Apr 14, 2026
Merged

fix(ci): replace yq with Python nav patching in publish-devnotes#548
andreatgretel merged 1 commit intomainfrom
andreatgretel/fix/publish-devnotes

Conversation

@andreatgretel
Copy link
Copy Markdown
Contributor

Summary

  • Replace yq JSON roundtrip with a Python script (.github/scripts/patch-devnotes-nav.py) that surgically replaces only the Dev Notes nav block in mkdocs.yml
  • The yq approach was mangling the entire file (indentation, quoting, comments), causing mike deploy to fail

Follow-up fix to #546

The yq JSON roundtrip was mangling the entire mkdocs.yml file
(indentation, quoting, comments), causing mike deploy to fail.

Extract a Python script that surgically replaces only the Dev Notes
nav block, leaving all other content byte-identical.
@andreatgretel andreatgretel requested a review from a team as a code owner April 14, 2026 18:59
@github-actions
Copy link
Copy Markdown
Contributor

Code Review: PR #548 — fix(ci): replace yq with Python nav patching in publish-devnotes

Summary

This PR replaces a yq-based JSON roundtrip in the publish-devnotes workflow with a purpose-built Python script (.github/scripts/patch-devnotes-nav.py). The yq approach was mangling the entire mkdocs.yml file (indentation, quoting, comments), causing mike deploy to fail. The new script surgically splices only the "Dev Notes" nav block from HEAD's mkdocs.yml into the older source checkout, leaving all other content byte-identical. This is a follow-up fix to #546.

Files changed: 2 (+60, −5)

  • .github/scripts/patch-devnotes-nav.py (new, 57 lines)
  • .github/workflows/publish-devnotes.yml (modified, 3 added / 5 deleted)

Findings

1. Block-end detection may stop too early on comment lines (Medium)

File: .github/scripts/patch-devnotes-nav.py:31

The while-loop that detects the end of the Dev Notes block checks whether a line starts with 6 spaces (" ") or " #" (a 6-space-indented comment). However, looking at the actual mkdocs.yml, the comment inside the Dev Notes block uses 6 spaces of indentation:

  - Dev Notes:
      # NOTE: Order is most recent -> oldest ...
      - devnotes/index.md

The condition on line 31 is:

if lines[end].strip() and not lines[end].startswith("      ") and not lines[end].startswith("      #"):

The comment # NOTE: ... starts with 6 spaces, so lines[end].startswith(" ") is already True (the first condition catches it). The explicit " #" check is redundant but harmless — the logic is correct for the current file structure. No bug here, just a minor redundancy.

2. Regex anchors match only exactly 2-space-indented - Dev Notes: (Low / Positive)

File: .github/scripts/patch-devnotes-nav.py:23

if re.match(r"^  - Dev Notes:", line):

This correctly matches the mkdocs.yml nav structure where top-level nav entries are indented 2 spaces. The anchoring is precise and avoids false matches. Good.

3. No trailing newline handling edge case (Low)

File: .github/scripts/patch-devnotes-nav.py:20

text.splitlines(keepends=True) is used, which preserves line endings. If the Dev Notes block is the very last section in the file and there's no trailing newline, the last line of the block won't have a \n. In practice, mkdocs.yml has content after the Dev Notes block (theme: section), so this is not an issue in the current layout. Just noting for robustness awareness.

4. Block boundary: empty lines are included in the block (Low)

File: .github/scripts/patch-devnotes-nav.py:31

The condition lines[end].strip() means empty/whitespace-only lines are included in the block (the if is False so the loop continues). In the actual mkdocs.yml, there's an empty line between the last Dev Notes entry and the theme: section. This means the empty line gets included in the extracted block from HEAD and replaces the corresponding gap in the target — which is correct behavior, preserving the blank separator line. Good design choice.

5. Workflow change is minimal and correct (Positive)

File: .github/workflows/publish-devnotes.yml:41,47-48

The workflow now checks out the Python script alongside docs/devnotes/ from HEAD, then uses it to patch mkdocs.yml. The yq install step is removed. The git show approach to get HEAD's mkdocs.yml into /tmp/ is clean and avoids an extra checkout.

6. No tests for the Python script (Low)

The script is a CI-only utility and relatively simple (57 lines). While it would be ideal to have a unit test for extract_devnotes_block, this is pragmatic for a CI fix script. The script is self-contained and its correctness can be verified by the workflow itself. Not blocking.

7. Script has proper SPDX headers and from __future__ import annotations (Positive)

Follows project conventions per CLAUDE.md.

Verdict

LGTM — This is a clean, well-scoped fix. The Python script is straightforward, correctly handles the mkdocs.yml nav structure, and eliminates the yq dependency that was corrupting the file. The block detection logic is sound for the current file layout. No blocking issues found.

@greptile-apps
Copy link
Copy Markdown
Contributor

greptile-apps Bot commented Apr 14, 2026

Greptile Summary

This PR replaces the yq JSON roundtrip in publish-devnotes.yml with a focused Python script (patch-devnotes-nav.py) that reads both the HEAD and target mkdocs.yml files, locates each - Dev Notes: block by regex and 6-space-indented child detection, and splices the HEAD block into the target in-place — preserving all other YAML formatting.

Confidence Score: 5/5

  • Safe to merge — the Python script correctly handles the actual mkdocs.yml indentation format and no P0/P1 issues were found.
  • The block-detection logic uses 6-space indentation, which exactly matches the live mkdocs.yml (confirmed by reading the file). The workflow correctly checks out the script from HEAD before invoking it, and the splice logic is straightforward. No logic errors or correctness issues were identified.
  • No files require special attention.

Important Files Changed

Filename Overview
.github/scripts/patch-devnotes-nav.py New Python script that surgically replaces only the Dev Notes nav block in mkdocs.yml; indentation assumptions (6-space children) match the actual file format.
.github/workflows/publish-devnotes.yml Drops yq install and JSON-roundtrip in favour of the new Python script; also checks out the script from HEAD alongside devnotes so it is always available.

Sequence Diagram

sequenceDiagram
    participant WF as publish-devnotes.yml
    participant Git as git
    participant Py as patch-devnotes-nav.py
    participant FS as mkdocs.yml (target)

    WF->>Git: checkout SOURCE_SHA (old source)
    WF->>Git: checkout github.sha -- docs/devnotes/ + patch script
    WF->>Git: "git show github.sha:mkdocs.yml > /tmp/mkdocs-head.yml"
    WF->>Py: python3 patch-devnotes-nav.py /tmp/mkdocs-head.yml mkdocs.yml
    Py->>Py: extract_devnotes_block(head) → head_block lines
    Py->>FS: extract_devnotes_block(target) → old block range
    Py->>FS: splice head_block into target, write back
    WF->>WF: mike deploy (uses patched mkdocs.yml)
Loading

Reviews (1): Last reviewed commit: "fix(ci): replace yq with Python nav patc..." | Re-trigger Greptile

@andreatgretel andreatgretel merged commit f267e19 into main Apr 14, 2026
51 checks passed
przemekboruta pushed a commit to przemekboruta/DataDesigner that referenced this pull request Apr 14, 2026
…DIA-NeMo#548)

The yq JSON roundtrip was mangling the entire mkdocs.yml file
(indentation, quoting, comments), causing mike deploy to fail.

Extract a Python script that surgically replaces only the Dev Notes
nav block, leaving all other content byte-identical.
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.

2 participants