Conversation
Fixes the three findings issue #1010 grouped as "lower priority than data integrity, but confirmed real and cheap to fix": 1. **Nested-bracket link-label regex gap** (PR #913): `spec/validate.py`'s and `spec/audit.py`'s `[^\]]*`-based link regexes stopped at the first `]`, so `[API [docs]](url)` passed both the registry description gate and `strip_md_links()` undetected. Replaced with a balanced bracket/paren scanner in both files. 2. **README PATH-persistence self-contradiction** (PR #964): the pre-commit snippet's README claimed `uv tool install` gives an unconditionally PATH-available command, contradicting the next sentence's own conditional-PATH guidance. Applied CodeRabbit's proposed wording. 3. **Quota-widening-only-when-empty gap** (PR #986): `copilot_history()` only widened past `HISTORY_PRS` when the narrow window came back fully empty, so a narrow window carrying only a Copilot comment (no formal review) returned early with no usable bot id, leaving a review just outside the window permanently unread. Widening is now keyed on whether a usable bot id was found, not on emptiness. Each fix carries a regression test. Full suite (849 tests), ruff format/check, mypy, prose_lint, and repo_gate (eol/eol-coverage) all pass. Closes #1010. 🤖 Generated with [Claude Code](https://claude.com/claude-code) <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Documentation** * Clarified installation guidance for persistent tools and independently running hooks. * **Bug Fixes** * Improved review history detection when recent activity contains comments but no usable review information. * Enhanced Markdown link validation for nested and escaped brackets and parentheses, while safely ignoring unbalanced links. * Improved validation performance for descriptions containing many unmatched brackets. * **Tests** * Added coverage for widened review-history searches and complex Markdown link formats. * Added regression coverage for large, malformed link patterns. <!-- end of auto-generated comment: release notes by coderabbit.ai -->
## What
A `command -v shellcheck` (or markdownlint, cspell, actionlint,
editorconfig-checker, shfmt,
PSScriptAnalyzer) miss reads as "not installed" unless the checker
already knows these tools
are deliberately never installed natively on a fleet host, per
GOVERNANCE.md "Running the
Linters Locally." Nothing sat at the point that check fails to say so,
and CODESTYLE.md's own
Bash paragraph pointed only at the `shell-codestyle` Skill rather than
at a runnable invocation.
## Fix
- **GOVERNANCE.md** "Running the Linters Locally": states the general
principle inline, that
none of these tools is installed natively by decision, so their absence
from `command -v` is
expected rather than evidence the check is unavailable.
- **CODESTYLE.md**: the Bash paragraph now points at that section
directly instead of only at
the Skill, so a reader who consults the carried instruction file gets a
runnable path without
chasing a link.
- **AGENTS.md**: added a routing-table row ("Running a lint or format
check locally, or a lint
tool missing from `command -v`") so the deterministic,
every-session-read entry point covers
this too, rather than depending solely on the right Skill firing at the
right moment.
Docs-only change; `prose_lint.py`, `repo_gate.py`, and the Docker
markdownlint/cspell lint all
pass clean.
Fixes #763.
🤖 Generated with [Claude Code](https://claude.com/claude-code)
<!-- This is an auto-generated comment: release notes by coderabbit.ai
-->
## Summary by CodeRabbit
* **Documentation**
* Clarified shell linting and formatting guidance.
* Documented the supported clean-compile workflow for environments where
required tools are not installed locally.
* Clarified that missing tools from `PATH` does not indicate that checks
are unavailable.
* Added guidance for locating and running local lint and formatting
checks consistently.
<!-- end of auto-generated comment: release notes by coderabbit.ai -->
|
Warning Review limit reachedNext included review available in 1 minute. View limit detailsLimit details: You’ve used all 10 included reviews currently available. Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. Review configuration: ⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Plus Run ID: 📒 Files selected for processing (9)
Comment |
PR Summary by QodoHarden Markdown link parsing and Copilot history lookup
AI Description
Diagram
High-Level Assessment
Files changed (9)
|
Code Review by Qodo
1. CODESTYLE.md duplicates lint rule
|
![qodo-code-review[bot]](https://avatars.githubusercontent.com/in/484649?s=60&v=4){kind=small}
| ## Shell | ||
|
|
||
| Bash, and only where a program cannot be Python: a bootstrap that installs the interpreter cannot be written in it, and a host tool that must run before a development toolchain exists cannot depend on one. Everything else is Python, with a test under the scripts tree's `tests/` directory. The mandatory `set -Eeuo pipefail` header, the pipefail-versus-early-reader pitfall, self-locating scripts, the `shellcheck`-plus-`shfmt` clean-compile, and the why-not-what comment rule are packaged as the `shell-codestyle` Skill at `.agents/skills/shell-codestyle/SKILL.md` in the hub, not a repo-relative link since that path is hub-local and not carried into every fleet repo. Read the skill for the full rules. | ||
| Bash, and only where a program cannot be Python: a bootstrap that installs the interpreter cannot be written in it, and a host tool that must run before a development toolchain exists cannot depend on one. Everything else is Python, with a test under the scripts tree's `tests/` directory. The mandatory `set -Eeuo pipefail` header, the pipefail-versus-early-reader pitfall, self-locating scripts, the `shellcheck`-plus-`shfmt` clean-compile, and the why-not-what comment rule are packaged as the `shell-codestyle` Skill at `.agents/skills/shell-codestyle/SKILL.md` in the hub, not a repo-relative link since that path is hub-local and not carried into every fleet repo. Read the skill for the full rules. Run the clean-compile check itself per [GOVERNANCE.md "Running the Linters Locally"][governance-running-the-linters-locally], not by probing `command -v shellcheck`. |
There was a problem hiding this comment.
1. codestyle.md duplicates lint rule 📘 Rule violation ⚙ Maintainability
The added sentence restates the canonical GOVERNANCE.md rule that a missing native shellcheck binary does not make the check unavailable. The non-canonical file should only point readers to the governing section.
Agent Prompt
## Issue description
`CODESTYLE.md` partially restates the canonical rule about handling a missing native lint binary.
## Issue Context
Cross-cutting conditions and obligations must remain in `AGENTS.md` or `GOVERNANCE.md`. Other files may reference the canonical section without repeating its substance.
## Fix Focus Areas
- CODESTYLE.md[54-54]
- GOVERNANCE.md[215-217]
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
Comment on lines
+50
to
+53
| A character class like `[^\\]]*` cannot count depth, so it stops at the first close and misses a link | ||
| label carrying its own nested brackets, e.g. `[API [docs]](url)`. Counting only open_char/close_char | ||
| nesting, ignoring the other bracket type, needs one such map per bracket type rather than one pass | ||
| mixing both. Built with a single left-to-right stack pass over the whole text rather than one depth- |
There was a problem hiding this comment.
2. Docstring sentences wrap across lines 📜 Skill insight ✧ Quality
The added _bracket_matches prose wraps individual sentences across several physical lines, including a split after a link. This violates the requirement that each sentence occupy exactly one line.
Agent Prompt
## Issue description
Sentences in the new helper docstrings are wrapped across physical lines.
## Issue Context
Multi-line comments and docstrings must contain exactly one complete sentence per line, without mid-sentence wrapping.
## Fix Focus Areas
- spec/validate.py[50-62]
- spec/audit.py[633-644]
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
Comment on lines
+53
to
+56
| mixing both. Built with a single left-to-right stack pass over the whole text rather than one depth- | ||
| counting scan per open position: re-scanning from every unmatched open is what made a prior version of | ||
| this walk O(N^2) on a run of N unmatched opens (#1011, CodeRabbit, on spec/audit.py's sibling | ||
| implementation). A close pops the most recently pushed open, the same pairing a fresh depth count from |
There was a problem hiding this comment.
4. _bracket_matches exposes implementation details 📜 Skill insight ✧ Quality
The new _bracket_matches docstrings are lengthy prose blocks that explain stack traversal, complexity history, repeated scans, and push/pop mechanics rather than concisely stating the callable's behavioral contract. Preserve only the mapping and escaping guarantees callers rely on, moving any indispensable algorithm rationale to narrowly scoped inline comments.
Agent Prompt
## Issue description
The new `_bracket_matches` docstrings are long explanatory prose blocks that describe internal algorithm mechanics and implementation history instead of presenting a concise behavioral contract.
## Issue Context
Docstrings should state what callers can rely on and remain concise. Preserve the helper's mapping and escaping contract, and remove implementation discussion about stack traversal, repeated scans, complexity history, and push/pop mechanics; if any rationale is indispensable, express it through clearer code structure or a narrowly scoped inline comment near the relevant code, using one line by default and a second only for a genuine constraint the code cannot carry.
## Fix Focus Areas
- spec/validate.py[48-63]
- spec/audit.py[631-645]
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
Comment on lines
+3492
to
+3493
| # A run of unmatched '[' used to re-scan the remaining text from every position (#1011, CodeRabbit), | ||
| # O(N^2) on a README tagline read before any length limit. Linear now: a slow run means a regression. |
There was a problem hiding this comment.
5. Comments use historical change framing 📜 Skill insight ✧ Quality
The new comments describe what the implementation used to do and call the new behavior `Linear now`. Documentation should state the current invariant directly rather than narrating the change.
Agent Prompt
## Issue description
Added comments and docstrings narrate prior behavior using phrases such as `used to`, `previous version`, and `Linear now`.
## Issue Context
Comments and documentation should describe current behavior in present tense. Historical before/after framing belongs in the PR description or changelog.
## Fix Focus Areas
- spec/audit.py[637-644]
- spec/audit.py[3473-3493]
- scripts/tests/test_spec_validate.py[193-246]
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
Comment on lines
+247
to
+249
| start = time.monotonic() | ||
| validate.contains_description_markdown_link("[" * 20000) | ||
| self.assertLess(time.monotonic() - start, 1.0) |
There was a problem hiding this comment.
6. Wall-clock checks can flake 🐞 Bug ☼ Reliability
The new regression test fails solely when one invocation takes over one wall-clock second, so a paused, contended, or coverage-instrumented CI process can fail even though _bracket_matches() remains linear. The same timing gate is duplicated in spec/audit.py --selftest, making the validation workflow vulnerable in two places.
Agent Prompt
## Issue description
The new complexity regression checks use a fixed one-second wall-clock threshold, which can fail nondeterministically when CI is paused or contended even if the implementation remains linear.
## Issue Context
Both the unit suite and `spec/audit.py --selftest` run under coverage in the validation workflow, adding variable instrumentation and host overhead. Keep regression coverage deterministic; move performance measurement to a non-gating benchmark or verify the algorithm without a wall-clock deadline.
## Fix Focus Areas
- scripts/tests/test_spec_validate.py[244-249]
- spec/audit.py[3492-3505]
- .github/actions/validate/action.yml[28-35]
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Promotes
developtomain.Includes:
🤖 Generated with Claude Code