fix(docs): tool-table completeness — 16 vanished tools restored + hard check - #42
Conversation
trigger/schedule/sprinkler/speedcontrol controls and query_sql_logger matched no group predicate and were absent from the README table for five releases (~16 tools). Predicates extended, History group added, and the generator now exits non-zero naming any tool that lands in zero or two groups. Closes #41. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01EYAGXd4bE9D9Z5kZeSHeo7
📝 WalkthroughWalkthroughThe tool documentation generator adds History classification, expands Control matching, and validates that every tool belongs to exactly one group. The README reflects the revised counts and sections, while plugin and package versions advance to 2026.8.1. ChangesTool documentation grouping
Estimated code review effort: 2 (Simple) | ~10 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
scripts/generate_tool_doc.py (1)
85-105: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winAdd regression coverage for the new grouping rules.
Cover every restored prefix, both History tools, Lookup exclusion, and zero/multiple-group failures so future predicate changes cannot silently alter the README count or classification.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@scripts/generate_tool_doc.py` around lines 85 - 105, Add regression tests for the grouping predicates and _GROUPS classification in generate_tool_doc.py, covering every restored _is_control prefix, both _HISTORY_TOOLS entries, and confirming history tools are excluded from _is_lookup_final. Also test that zero-group and multiple-group matches fail, preserving the expected README tool count and classification behavior.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@scripts/generate_tool_doc.py`:
- Around line 131-140: Move the completeness validation using placements and bad
in the tool-document generation flow so it runs before the first stdout print.
Ensure any validation failure exits without emitting the generated header or
partial README content, while preserving the existing exactly-one-group check.
- Around line 135-136: Rename the single-letter comprehension bindings in the
placements construction to descriptive names, using labels such as label and
predicate for _GROUPS entries and retaining clear names for tool names and group
collections. Update only the affected comprehensions while preserving the
existing placements and bad calculations.
---
Nitpick comments:
In `@scripts/generate_tool_doc.py`:
- Around line 85-105: Add regression tests for the grouping predicates and
_GROUPS classification in generate_tool_doc.py, covering every restored
_is_control prefix, both _HISTORY_TOOLS entries, and confirming history tools
are excluded from _is_lookup_final. Also test that zero-group and multiple-group
matches fail, preserving the expected README tool count and classification
behavior.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: c51cf847-3408-4c21-be48-c49b93078418
📒 Files selected for processing (4)
Indigo MCP Lite.indigoPlugin/Contents/Info.plistREADME.mdpyproject.tomlscripts/generate_tool_doc.py
| # Completeness check: every tool must land in EXACTLY one group. | ||
| # A tool matching none silently vanished from the README for five | ||
| # releases (trigger/schedule/sprinkler/speedcontrol/query_sql_logger) | ||
| # — fail loudly instead. | ||
| placements = {name: [l for l, p in _GROUPS if p(name)] for name, _ in tools} | ||
| bad = {n: ls for n, ls in placements.items() if len(ls) != 1} | ||
| if bad: | ||
| import sys | ||
| print(f"ERROR: tools not in exactly one group: {bad}", file=sys.stderr) | ||
| sys.exit(1) |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick win
Validate group coverage before emitting output.
The script prints the generated header before checking completeness. If stdout is redirected into README.md, a failed run can leave a partial generated document. Move this validation before the first print(...), or stage output and replace the file only after success.
🧰 Tools
🪛 Ruff (0.15.21)
[error] 135-135: Ambiguous variable name: l
(E741)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@scripts/generate_tool_doc.py` around lines 131 - 140, Move the completeness
validation using placements and bad in the tool-document generation flow so it
runs before the first stdout print. Ensure any validation failure exits without
emitting the generated header or partial README content, while preserving the
existing exactly-one-group check.
| placements = {name: [l for l, p in _GROUPS if p(name)] for name, _ in tools} | ||
| bad = {n: ls for n, ls in placements.items() if len(ls) != 1} |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "Repository files matching generate_tool_doc.py:"
git ls-files | rg '(^|/)generate_tool_doc\.py$' || true
file="$(git ls-files | rg '(^|/)generate_tool_doc\.py$' | head -n1)"
if [ -n "${file:-}" ]; then
echo
echo "File: $file"
echo "Line count:"
wc -l "$file"
echo
echo "Relevant lines 125-145:"
sed -n '125,145p' "$file" | nl -ba -v125
echo
echo "Imports/top context:"
sed -n '1,80p' "$file" | nl -ba -v1
fi
echo
echo "Search for Ruff config and E741 references:"
git ls-files | rg '(^|/)(pyproject\.toml|setup\.cfg|ruff\.toml|\.ruff\.toml)$' || true
rg -n "E741|ruff|select|ignore" -S pyproject.toml setup.cfg ruff.toml .ruff.toml 2>/dev/null || trueRepository: simons-plugins/indigo-mcp-lite
Length of output: 392
🏁 Script executed:
#!/bin/bash
set -euo pipefail
file="scripts/generate_tool_doc.py"
echo "Relevant lines 125-145:"
awk 'NR>=125 && NR<=145 {printf "%4d: %s\n", NR, $0}' "$file"
echo
echo "Imports/top context:"
awk 'NR>=1 && NR<=80 {printf "%4d: %s\n", NR, $0}' "$file"
echo
echo "Search for Ruff config:"
git ls-files | awk 'tolower($0) ~ /(^(|.*\/)(pyproject\.toml|setup\.cfg|ruff\.toml|\.ruff\.toml))$/ {print}'
echo
echo "Ruff/E741 references in configs:"
for f in $(git ls-files | awk 'tolower($0) ~ /(^(|.*\/)(pyproject\.toml|setup\.cfg|ruff\.toml|\.ruff\.toml))$/ {print}'); do
echo "--- $f"
grep -InEi 'E741|ruff|select|ignore' "$f" || true
doneRepository: simons-plugins/indigo-mcp-lite
Length of output: 4380
🌐 Web query:
Ruff rule E741 ambiguous variable name l
💡 Result:
The Ruff lint rule E741, titled ambiguous-variable-name, flags the use of the characters 'l', 'O', or 'I' as variable names [1][2]. This rule is based on PEP 8 recommendations, which advise against using these specific characters because they can be easily confused with numerals in many fonts: - 'l' (lowercase L) can look like '1' (one). - 'O' (uppercase O) can look like '0' (zero). - 'I' (uppercase I) can look like '1' (one) or a lowercase 'l' depending on the font [1][3]. The rule suggests using more descriptive names or alternative identifiers (e.g., using 'L' instead of 'l') [1][3]. Important details regarding this rule in Ruff: - Rule Scope: It is automatically disabled for stub files (files with.pyi extensions), as stub authors often need to mirror the exact interface of the runtime module, including any legacy naming choices [1][4]. - Configuration: As with other lint rules, you can ignore E741 on a per-line basis using # noqa: E741, or configure it via the lint.ignore setting in your Ruff configuration file [5][6].
Citations:
- 1: https://docs.astral.sh/ruff/rules/ambiguous-variable-name/
- 2: https://github.com/astral-sh/ruff/blob/main/crates%2Fruff_linter%2Fsrc%2Frules%2Fpycodestyle%2Frules%2Fambiguous_variable_name.rs
- 3: https://www.flake8rules.com/rules/E741.html
- 4: Make E741 not apply to stub files astral-sh/ruff#10569
- 5: https://docs.astral.sh/ruff/linter/
- 6: https://github.com/astral-sh/ruff/blob/56eb6b62/docs/linter.md
Rename the ambiguous comprehension variables.
Ruff’s E741 rule flags single-letter bindings like l/O/I; use descriptive names such as label, predicate, name, and groups.
Proposed fix
- placements = {name: [l for l, p in _GROUPS if p(name)] for name, _ in tools}
- bad = {n: ls for n, ls in placements.items() if len(ls) != 1}
+ placements = {
+ name: [label for label, predicate in _GROUPS if predicate(name)]
+ for name, _ in tools
+ }
+ bad = {name: groups for name, groups in placements.items() if len(groups) != 1}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| placements = {name: [l for l, p in _GROUPS if p(name)] for name, _ in tools} | |
| bad = {n: ls for n, ls in placements.items() if len(ls) != 1} | |
| placements = { | |
| name: [label for label, predicate in _GROUPS if predicate(name)] | |
| for name, _ in tools | |
| } | |
| bad = {name: groups for name, groups in placements.items() if len(groups) != 1} |
🧰 Tools
🪛 Ruff (0.15.21)
[error] 135-135: Ambiguous variable name: l
(E741)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@scripts/generate_tool_doc.py` around lines 135 - 136, Rename the
single-letter comprehension bindings in the placements construction to
descriptive names, using labels such as label and predicate for _GROUPS entries
and retaining clear names for tool names and group collections. Update only the
affected comprehensions while preserving the existing placements and bad
calculations.
Source: Linters/SAST tools
1 participant
Closes #41. All 71 tools now render (was ~55); the generator fails loudly if any registered tool lands in zero or two groups, so this bug class is dead. 2026.8.0 → 2026.8.1.
🤖 Generated with Claude Code
https://claude.ai/code/session_01EYAGXd4bE9D9Z5kZeSHeo7
Summary by CodeRabbit
New Features
Documentation
Chores