Conversation
…or external URLs referenced in the repo and check validity
|
Warning Rate limit exceeded
⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughThis PR updates documentationLink values across multiple board and configuration files and adds a new repository URL validation script that extracts, classifies, and tests HTTP/HTTPS links found in tracked files. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant DevMachine as Developer / CI
participant Repo as Git Repository (files)
participant Checker as check-urls.py
participant Remote as Remote HTTP Servers
DevMachine->>Repo: git ls-files -> tracked files list
Checker->>Repo: read files (exclude patterns)
Checker->>Checker: extract URLs, classify (interpolated/query/testable)
Checker->>Remote: HEAD request (testable URLs)
alt HEAD 405
Checker->>Remote: GET request
end
Remote-->>Checker: HTTP status / error
Checker->>DevMachine: aggregated report (pass/fail, locations)
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (5)
scripts/README.md (2)
7-7: Heading level inconsistency.Changed from
###to##forvalidate-bash-syntax.sh, but this creates a peer heading with "Available Scripts" rather than being nested under it. Consider using###to maintain proper document hierarchy.♻️ Suggested fix
-## `validate-bash-syntax.sh` +### `validate-bash-syntax.sh`🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@scripts/README.md` at line 7, The heading for validate-bash-syntax.sh is at the wrong level; change the `##` heading back to `###` so `validate-bash-syntax.sh` is nested under the "Available Scripts" section (match the other script headings), i.e., locate the line containing the literal heading text `validate-bash-syntax.sh` and update its markdown prefix from double- to triple-hash to restore proper document hierarchy.
157-160: New script documentation added.The documentation for
check-urls.pyis concise and includes a helpful tip about using VSCode terminal for clickable links.Minor: Consider capitalizing "vscode" as "VS Code" or "VSCode" for consistency with the product name.
♻️ Suggested fix
-Best run from a vscode terminal, as you can then click on the detected URLs and file references in the output. +Best run from a VS Code terminal, as you can then click on the detected URLs and file references in the output.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@scripts/README.md` around lines 157 - 160, In the README paragraph describing `check-urls.py`, update the sentence that currently reads "Best run from a vscode terminal, as you can then click on the detected URLs and file references in the output." to use the correct product capitalization (e.g., "Best run from a VS Code terminal" or "Best run from a VSCode terminal"); modify the markdown text surrounding `check-urls.py` so the phrase "vscode" is replaced with the chosen capitalization to keep documentation consistent.scripts/check-urls.py (3)
7-7: EN DASH character used instead of standard hyphen.The docstring and several comments use EN DASH (
–, U+2013) instead of HYPHEN-MINUS (-, U+002D). While functional, using standard ASCII hyphens improves consistency and avoids potential copy-paste issues.Affected locations include line 7 (docstring), and various comments throughout the file flagged by static analysis.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@scripts/check-urls.py` at line 7, The module docstring and comments in scripts/check-urls.py use the EN DASH (U+2013) instead of the standard hyphen-minus (U+002D); search the file (including the module docstring and any inline comments) for the EN DASH character and replace each occurrence with a normal ASCII hyphen '-' to standardize formatting and avoid copy/paste issues—after replacing, run your linter or a quick grep for U+2013 to confirm all instances are fixed.
281-307: Consider session reuse for better performance.A new
requests.Sessionis created for each URL check. While this is thread-safe, it's less efficient due to repeated connection setup overhead. For a utility script this is acceptable, but if performance becomes a concern, you could use a thread-local session.♻️ Optional optimization using thread-local sessions
+import threading + +_thread_local = threading.local() + +def _get_session() -> requests.Session: + if not hasattr(_thread_local, "session"): + _thread_local.session = _make_session() + return _thread_local.session + def check_url(url: str, timeout: int) -> tuple[str, Optional[int], Optional[str]]: - session = _make_session() + session = _get_session()🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@scripts/check-urls.py` around lines 281 - 307, The check_url function currently calls _make_session() for each request which creates a new requests.Session per URL; change _make_session (or add a helper like _get_thread_session) to return a thread-local cached requests.Session using threading.local so each thread reuses a single Session (preserving thread-safety) instead of allocating a new one every call; update check_url to call that thread-local getter and ensure any session-level headers (USER_AGENT) are set once when the session is created.
351-362: URLs with query strings are excluded from reachability testing.This design choice means URLs like
https://example.com/docs?version=2will only be reported but not validated. Consider whether this is the intended behavior, as some legitimate documentation URLs may include query parameters.If excluding query strings is intentional (e.g., to avoid unintended side effects from GET requests with parameters), this is fine. Otherwise, you could test the base URL portion.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@scripts/check-urls.py` around lines 351 - 362, The current classification loop sends any UrlInfo with a "?" in its url into query_infos, excluding them from reachability testing; decide whether to include such URLs by updating the loop in the classification block that iterates over url_map.values() — either (A) treat query URLs as testable by appending them to test_infos instead of query_infos, or (B) normalize by splitting info.url at the "?" to get the base URL and create/modify a UrlInfo for that base and append it to test_infos (while optionally keeping the original in query_infos for reporting); update the logic around interpolated_infos, query_infos, and test_infos accordingly so query-parametered URLs are validated as intended.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@configuration/klippy/beacon_mesh.py`:
- Around line 342-344: The user-facing help string in the variable link_line
contains a typo ("Lean more about")—update the value of link_line to read "Learn
more about" while preserving the existing HTML and variables (link_url,
link_text); locate and edit the assignment to link_line so the displayed message
becomes 'Learn more about <a href="{link_url}" target="_blank">{link_text}</a>'.
---
Nitpick comments:
In `@scripts/check-urls.py`:
- Line 7: The module docstring and comments in scripts/check-urls.py use the EN
DASH (U+2013) instead of the standard hyphen-minus (U+002D); search the file
(including the module docstring and any inline comments) for the EN DASH
character and replace each occurrence with a normal ASCII hyphen '-' to
standardize formatting and avoid copy/paste issues—after replacing, run your
linter or a quick grep for U+2013 to confirm all instances are fixed.
- Around line 281-307: The check_url function currently calls _make_session()
for each request which creates a new requests.Session per URL; change
_make_session (or add a helper like _get_thread_session) to return a
thread-local cached requests.Session using threading.local so each thread reuses
a single Session (preserving thread-safety) instead of allocating a new one
every call; update check_url to call that thread-local getter and ensure any
session-level headers (USER_AGENT) are set once when the session is created.
- Around line 351-362: The current classification loop sends any UrlInfo with a
"?" in its url into query_infos, excluding them from reachability testing;
decide whether to include such URLs by updating the loop in the classification
block that iterates over url_map.values() — either (A) treat query URLs as
testable by appending them to test_infos instead of query_infos, or (B)
normalize by splitting info.url at the "?" to get the base URL and create/modify
a UrlInfo for that base and append it to test_infos (while optionally keeping
the original in query_infos for reporting); update the logic around
interpolated_infos, query_infos, and test_infos accordingly so query-parametered
URLs are validated as intended.
In `@scripts/README.md`:
- Line 7: The heading for validate-bash-syntax.sh is at the wrong level; change
the `##` heading back to `###` so `validate-bash-syntax.sh` is nested under the
"Available Scripts" section (match the other script headings), i.e., locate the
line containing the literal heading text `validate-bash-syntax.sh` and update
its markdown prefix from double- to triple-hash to restore proper document
hierarchy.
- Around line 157-160: In the README paragraph describing `check-urls.py`,
update the sentence that currently reads "Best run from a vscode terminal, as
you can then click on the detected URLs and file references in the output." to
use the correct product capitalization (e.g., "Best run from a VS Code terminal"
or "Best run from a VSCode terminal"); modify the markdown text surrounding
`check-urls.py` so the phrase "vscode" is replaced with the chosen
capitalization to keep documentation consistent.
ℹ️ Review info
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (17)
configuration/boards/ay-caramba/board-definition.jsonconfiguration/boards/btt-ebb36-10/board-definition.jsonconfiguration/boards/btt-ebb36-11/board-definition.jsonconfiguration/boards/btt-ebb36-12/board-definition.jsonconfiguration/boards/btt-ebb42-10/board-definition.jsonconfiguration/boards/btt-ebb42-11/board-definition.jsonconfiguration/boards/btt-ebb42-12/board-definition.jsonconfiguration/boards/btt-ebb42-gen2-10/board-definition.jsonconfiguration/boards/btt-manta-m4p-22/board-definition.jsonconfiguration/boards/btt-manta-m4p/board-definition.jsonconfiguration/boards/btt-octopus-pro-h723-11/board-definition.jsonconfiguration/klippy/beacon_mesh.pyconfiguration/macros.cfgconfiguration/printers/v-core-3-idex/macros.cfgconfiguration/printers/v-core-3/macros.cfgscripts/README.mdscripts/check-urls.py
* feat(scripts): add check-urls.py, a development-time script to scan for external URLs referenced in the repo and check validity * fix(urls): fix various invalid external URLs, mostly links to documentation * chore: fix typo * chore: resolve heading level inconsistency
Summary by CodeRabbit
Documentation
Chores