-
Notifications
You must be signed in to change notification settings - Fork 54
LCORE-578: Script to fix OpenAPI documentation generation #464
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
LCORE-578: Script to fix OpenAPI documentation generation #464
Conversation
WalkthroughMakefile's openapi-doc now produces an intermediate Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor Dev as Developer
participant Make as Makefile (openapi-doc)
participant O2M as openapi-to-markdown
participant Fix as scripts/fix_openapi_doc.py
participant FS as Filesystem
Dev->>Make: make openapi-doc
Make->>O2M: openapi-to-markdown --input_file docs/openapi.json --output_file output.md
O2M-->>FS: write output.md
Note over Make,Fix: post-process intermediate file
Make->>Fix: python3 scripts/fix_openapi_doc.py < output.md > docs/output.md
Fix-->>FS: write docs/output.md
Make->>FS: rm output.md
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Poem
✨ Finishing Touches
🧪 Generate unit tests
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 2
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (2)
Makefile(1 hunks)scripts/fix_openapi_doc.py(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: build-pr
- GitHub Check: e2e_tests
| openapi-doc: docs/openapi.json ## Generate OpenAPI documentation | ||
| openapi-to-markdown --input_file docs/openapi.json --output_file docs/output.md | ||
| openapi-to-markdown --input_file docs/openapi.json --output_file output.md | ||
| scripts/fix_openapi_doc.py < output.md > docs/output.md | ||
| rm output.md |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
💡 Verification agent
🧩 Analysis chain
Harden the pipeline: tmpfile, ensure docs/, and avoid exec-bit assumptions
Use a mktemp-managed file, ensure docs exists, and invoke the script via Python to avoid relying on executable bit. Also add the script as a dependency so changes retrigger the target.
Apply this diff:
-openapi-doc: docs/openapi.json ## Generate OpenAPI documentation
- openapi-to-markdown --input_file docs/openapi.json --output_file output.md
- scripts/fix_openapi_doc.py < output.md > docs/output.md
- rm output.md
+openapi-doc: docs/openapi.json scripts/fix_openapi_doc.py ## Generate OpenAPI documentation
+ @tmp=$$(mktemp); \
+ openapi-to-markdown --input_file docs/openapi.json --output_file $$tmp && \
+ mkdir -p docs && \
+ uv run python scripts/fix_openapi_doc.py < "$$tmp" > docs/output.md && \
+ rm -f "$$tmp"Quick checks to run:
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Ensure CLI exists
command -v openapi-to-markdown || { echo "openapi-to-markdown not found in PATH"; exit 1; }
# Show git mode for the script (755 preferred but no longer required after change)
git ls-files -s scripts/fix_openapi_doc.pyLength of output: 146
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Show git mode for the script
git ls-files -s scripts/fix_openapi_doc.py || true
# Search for references to the CLI in the repo
grep -R "openapi-to-markdown" -n . || trueLength of output: 266
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Verify 'uv' runner exists
if ! command -v uv >/dev/null; then
echo "uv not found in PATH"
else
echo "uv found at $(command -v uv)"
fi
# Verify a Python interpreter exists
if command -v python3 >/dev/null; then
echo "python3 found at $(command -v python3)"
elif command -v python >/dev/null; then
echo "python found at $(command -v python)"
else
echo "no Python interpreter found"
fiLength of output: 206
Harden openapi-doc Makefile target
Add scripts/fix_openapi_doc.py to the target prerequisites, use mktemp for the intermediate file, pre-create docs/ with mkdir -p, invoke the Python script via python3 (to avoid missing uv or relying on exec bits), and document/install openapi-to-markdown as a prerequisite.
🤖 Prompt for AI Agents
Makefile lines 39-42: the openapi-doc target is brittle—add
scripts/fix_openapi_doc.py into the target prerequisites, ensure docs/ exists by
running mkdir -p docs before writing, replace the fixed intermediate filename
with a secure mktemp-generated temp file and remove it afterward, invoke the
fixer via python3 scripts/fix_openapi_doc.py < temp > docs/output.md (avoid
relying on executable bit), and document/require openapi-to-markdown (e.g., add
a comment or a .PHONY/prereq note) so users know to install it.
| for i in range(lines_count): | ||
| current_line = lines[i].rstrip() | ||
|
|
||
| # limit check | ||
| if i == lines_count - 1: | ||
| print(current_line) | ||
| break | ||
|
|
||
| # the next line must exists -> read it | ||
| next_line = lines[i + 1].rstrip() | ||
|
|
||
| # skip the continuation line as it was used already | ||
| # during previous line processing | ||
| if current_line == TABLE_ROW_CONTINUATION_LINE: | ||
| continue | ||
|
|
||
| # check if table row continuation line is present | ||
| if next_line == " |": | ||
| print(current_line + next_line) | ||
| else: | ||
| print(current_line) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
💡 Verification agent
🧩 Analysis chain
Fix last-line duplication and preserve Markdown spaces
If the final line is the continuation marker " |", it gets printed twice due to the last-line check preceding the skip check. Also, using rstrip() drops significant trailing spaces (Markdown line breaks). Compare with the constant instead of a string literal.
Apply this diff:
for i in range(lines_count):
- current_line = lines[i].rstrip()
+ current_line = lines[i].rstrip("\r\n")
- # limit check
- if i == lines_count - 1:
- print(current_line)
- break
-
- # the next line must exists -> read it
- next_line = lines[i + 1].rstrip()
-
- # skip the continuation line as it was used already
- # during previous line processing
- if current_line == TABLE_ROW_CONTINUATION_LINE:
- continue
+ # skip the continuation line as it was used already
+ # during previous line processing
+ if current_line == TABLE_ROW_CONTINUATION_LINE:
+ continue
+
+ # limit check
+ if i == lines_count - 1:
+ print(current_line)
+ break
+
+ # the next line must exist -> read it
+ next_line = lines[i + 1].rstrip("\r\n")
# check if table row continuation line is present
- if next_line == " |":
- print(current_line + next_line)
+ if next_line == TABLE_ROW_CONTINUATION_LINE:
+ print(current_line + TABLE_ROW_CONTINUATION_LINE)
else:
print(current_line)Reproduce and see the current bug (prints " |" twice); after patch, it should print once:
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf 'row content\n |\n' > /tmp/in.md
echo "Actual:"
python3 scripts/fix_openapi_doc.py < /tmp/in.md
echo "Expected:"
echo 'row content |'Length of output: 189
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Display original code snippet
echo "Original code snippet:"
sed -n '1,200p' scripts/fix_openapi_doc.py
# Apply patch
patch -p0 << 'EOF'
*** Begin Patch
*** Update File: scripts/fix_openapi_doc.py
@@ for i in range(lines_count):
- current_line = lines[i].rstrip()
+ current_line = lines[i].rstrip("\r\n")
@@
- # limit check
- if i == lines_count - 1:
- print(current_line)
- break
-
- # the next line must exists -> read it
- next_line = lines[i + 1].rstrip()
-
- # skip the continuation line as it was used already
- # during previous line processing
- if current_line == TABLE_ROW_CONTINUATION_LINE:
- continue
+ # skip the continuation line as it was used already
+ # during previous line processing
+ if current_line == TABLE_ROW_CONTINUATION_LINE:
+ continue
+
+ # limit check
+ if i == lines_count - 1:
+ print(current_line)
+ break
+
+ # the next line must exist -> read it
+ next_line = lines[i + 1].rstrip("\r\n")
@@
- # check if table row continuation line is present
- if next_line == " |":
- print(current_line + next_line)
+ # check if table row continuation line is present
+ if next_line == TABLE_ROW_CONTINUATION_LINE:
+ print(current_line + TABLE_ROW_CONTINUATION_LINE)
*** End Patch
EOF
# Test the script after patch
printf 'row content\n |\n' > /tmp/in.md
echo "Actual after patch:"
python3 scripts/fix_openapi_doc.py < /tmp/in.md
echo "Expected:"
echo 'row content |'Length of output: 940
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "Inspect scripts/fix_openapi_doc.py (with visible whitespace markers):"
nl -ba scripts/fix_openapi_doc.py | sed -n '1,80p' | sed -e 's/ /·/g' -e 's/\t/→/g' -e 's/$/¶/'Length of output: 357
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "Showing lines 1-50 with visible whitespace markers:"
head -n 50 scripts/fix_openapi_doc.py | sed -e 's/ /·/g' -e 's/\t/→/g' -e 's/$/¶/'Length of output: 1006
Fix last-line duplication and preserve Markdown spaces
Reorder the continuation-skip and end-of-file checks to avoid printing the final TABLE_ROW_CONTINUATION_LINE twice, use rstrip("\r\n") to retain significant trailing spaces, and compare against the constant instead of a string literal.
for i in range(lines_count):
- current_line = lines[i].rstrip()
+ current_line = lines[i].rstrip("\r\n")
# skip the continuation line as it was used already
# during previous line processing
if current_line == TABLE_ROW_CONTINUATION_LINE:
continue
# limit check
if i == lines_count - 1:
print(current_line)
break
# the next line must exist -> read it
- next_line = lines[i + 1].rstrip()
+ next_line = lines[i + 1].rstrip("\r\n")
# check if table row continuation line is present
- if next_line == " |":
- print(current_line + next_line)
+ if next_line == TABLE_ROW_CONTINUATION_LINE:
+ print(current_line + TABLE_ROW_CONTINUATION_LINE)
else:
print(current_line)Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In scripts/fix_openapi_doc.py around lines 13 to 33, reorder the checks so the
continuation-skip runs before the end-of-file check, use lines[i].rstrip("\r\n")
and lines[i+1].rstrip("\r\n") to preserve meaningful trailing spaces, and
compare next_line against the TABLE_ROW_CONTINUATION_LINE constant (not the
literal " |"); when next_line equals that constant, print the concatenated
current_line + next_line, otherwise print current_line — this prevents the final
TABLE_ROW_CONTINUATION_LINE from being printed twice and preserves Markdown
spacing.
b250495 to
dea8fd9
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
♻️ Duplicate comments (1)
scripts/fix_openapi_doc.py (1)
13-33: Fix last-line duplication and add optional blank line after tables (MD058).Reorder checks so continuation-only lines aren’t printed twice, then insert one blank line after a table ends (only when the next line isn’t a table row or already blank). Also fix a small grammar nit in the comment.
for i in range(lines_count): - current_line = lines[i].rstrip("\r\n") - - # limit check - if i == lines_count - 1: - print(current_line) - break - - # the next line must exists -> read it - next_line = lines[i + 1].rstrip("\r\n") - - # skip the continuation line as it was used already - # during previous line processing - if current_line == TABLE_ROW_CONTINUATION_LINE: - continue - - # check if table row continuation line is present - if next_line == TABLE_ROW_CONTINUATION_LINE: - print(current_line + TABLE_ROW_CONTINUATION_LINE) - else: - print(current_line) + current_line = lines[i].rstrip("\r\n") + + # skip the continuation line as it was used already during previous line processing + if current_line == TABLE_ROW_CONTINUATION_LINE: + continue + + # limit check + if i == lines_count - 1: + print(current_line) + break + + # the next line must exist -> read it + next_line = lines[i + 1].rstrip("\r\n") + + # check if table row continuation line is present + if next_line == TABLE_ROW_CONTINUATION_LINE: + merged = current_line + TABLE_ROW_CONTINUATION_LINE + print(merged) + # ensure one blank line after the table if the following line is not a table row or blank + if i + 2 < lines_count: + next_next_line = lines[i + 2].rstrip("\r\n") + if not next_next_line.startswith("|") and next_next_line.strip() != "": + print() + else: + print(current_line) + # ensure one blank line after the table if current line is a table row and the next is not + if current_line.startswith("|") and not next_line.startswith("|") and next_line.strip() != "": + print()Quick repro to ensure no duplicate trailing " |" and that a blank line is added after table:
#!/bin/bash set -euo pipefail cat > /tmp/in.md <<'EOF' | H | D | |----|----| | 200 | OK | | ## Next EOF echo "Actual:" python3 scripts/fix_openapi_doc.py < /tmp/in.md | sed -n '1,20p' | nl -ba echo "Expected (note the single merged row and a blank line before ## Next):" cat <<'OUT' | nl -ba | H | D | |----|----| | 200 | OK | ## Next OUT
🧹 Nitpick comments (3)
docs/output.md (3)
29-30: Add a blank line after the table to satisfy MD058 and avoid grammar false-positives.Table rows should be surrounded by blank lines. Insert one after the last row.
-| 200 | Successful Response | string | +| 200 | Successful Response | string | +
50-51: Add a blank line after the table.Keeps markdownlint (MD058) happy and prevents headings from sticking to tables.
-| 200 | Successful Response | [InfoResponse](#inforesponse) | +| 200 | Successful Response | [InfoResponse](#inforesponse) | +
396-397: Add a blank line after each Markdown table
Missing blank lines were detected after tables ending at lines 29, 50, 76, 111, 146, 168, 207, 228, 244, 281, 312, 332, 351, 374, and 396. Insert an empty line after each table to satisfy MD058.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (3)
Makefile(1 hunks)docs/output.md(3 hunks)scripts/fix_openapi_doc.py(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- Makefile
🧰 Additional context used
🪛 LanguageTool
docs/output.md
[grammar] ~29-~29: There might be a mistake here.
Context: ...| | 200 | Successful Response | string | ## GET /v1/info > **Info Endpoint Handle...
(QB_NEW_EN)
[grammar] ~50-~50: There might be a mistake here.
Context: ...sponse | InfoResponse | ## GET /v1/models > **Models Endpoint Ha...
(QB_NEW_EN)
🪛 markdownlint-cli2 (0.17.2)
docs/output.md
29-29: Tables should be surrounded by blank lines
(MD058, blanks-around-tables)
50-50: Tables should be surrounded by blank lines
(MD058, blanks-around-tables)
396-396: Tables should be surrounded by blank lines
(MD058, blanks-around-tables)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: build-pr
Description
LCORE-578: Script to fix OpenAPI documentation generation
Type of change
Related Tickets & Documents
Summary by CodeRabbit
Documentation
Chores