Doc-gate expansion: CHANGELOG + modifications + broader agent-doc coverage - #2372
Conversation
… coverage, trailer logging - routes, installers, app-catalog, auth_middleware now trigger on plain modifications (on_modify = true) - user-visible-changelog broadened to cover all changes under tinyagentos/ and desktop/src/ except test-only - agent-manual rule now covers agent_scope_requests_store.py for agent identity/scope surface - Docs-Reviewed trailer override is logged in CI with commit hash, author, and reason - added tests for modification-triggers-gate, changelog-required, test-only-exempt, trailer-logged - updated CONTRIBUTING.md and CHANGELOG.md
|
ⓘ Qodo reviews are paused because the subscription is no longer active. Ask your workspace admin to reactivate the subscription to resume reviews. Manage billing |
📝 WalkthroughWalkthroughThe PR expands documentation-gate coverage to modified behavior-bearing files and broad source trees. It documents exemptions and trailer overrides, adds changelog entries, and logs populated ChangesDocumentation Gate
Estimated code review effort: 3 (Moderate) | ~20 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Warning Tools execution failed with the following error: Failed to run tools: 14 UNAVAILABLE: read ECONNRESET 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: 5
🤖 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 `@CONTRIBUTING.md`:
- Around line 293-295: Update the fenced code block in the commit-message
example to include the Markdown language identifier text, preserving the
existing example content.
- Around line 278-279: Update CONTRIBUTING.md lines 278-279 to state that either
an edit under docs/agent-manual/*.md or docs/agent-coordination.md satisfies the
agent-facing documentation rule. Update CHANGELOG.md lines 21-23 to document
both allowed paths and remove or verify the unsupported claim that
agent_auth_requests.py triggers the rule, while retaining agent_token_auth.py
only if accurate.
In `@docs/doc-gate.toml`:
- Around line 95-97: Align the changelog-rule documentation with its configured
non-test path scope: update docs/doc-gate.toml lines 95-97, CONTRIBUTING.md
lines 273-276, and CHANGELOG.md lines 17-19 to describe every non-test change
under tinyagentos/** or desktop/src/**, rather than only code changes. No rule
implementation change is needed.
In `@scripts/check_doc_gate.py`:
- Around line 320-338: Fix commit parsing in the flow that invokes _run_git and
builds commits by using separate record and field delimiters, then split each
record into exactly the hash, author, and message fields so evaluate_rules
receives valid trailers. Add a regression test through
_git_commits_with_messages or main verifying base-mode Docs-Reviewed: trailers
are evaluated and logged.
In `@tests/test_doc_gate.py`:
- Around line 459-462: Update the fixture path in tests/test_doc_gate.py lines
459-462 to a route-matching path such as tinyagentos/routes/test_agents.py, and
update the fixture in lines 511-514 to a broad-rule path such as
tinyagentos/test_agents.py, so both tests exercise _is_test_path; no other
changes are needed.
🪄 Autofix
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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 5552c7e9-5442-42ad-b78d-5a6f61d59f6c
📒 Files selected for processing (5)
CHANGELOG.mdCONTRIBUTING.mddocs/doc-gate.tomlscripts/check_doc_gate.pytests/test_doc_gate.py
| Agent-facing changes (agent registry, token auth, scope requests, agent routes, | ||
| MCP surface) also require touching the agent manual under `docs/agent-manual/`. |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Align the agent-facing documentation with agent-manual configuration. The configured rule accepts either docs/agent-manual/*.md or docs/agent-coordination.md. It triggers agent_token_auth.py, but it does not list agent_auth_requests.py.
CONTRIBUTING.md#L278-L279: state that an agent-manual edit ordocs/agent-coordination.mdsatisfies the rule.CHANGELOG.md#L21-L23: state the allowed documentation alternatives and remove or verify the unsupportedagent_auth_requests.pytrigger claim.
📍 Affects 2 files
CONTRIBUTING.md#L278-L279(this comment)CHANGELOG.md#L21-L23
🤖 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 `@CONTRIBUTING.md` around lines 278 - 279, Update CONTRIBUTING.md lines 278-279
to state that either an edit under docs/agent-manual/*.md or
docs/agent-coordination.md satisfies the agent-facing documentation rule. Update
CHANGELOG.md lines 21-23 to document both allowed paths and remove or verify the
unsupported claim that agent_auth_requests.py triggers the rule, while retaining
agent_token_auth.py only if accurate.
| # Broadened to cover every code change under tinyagentos/ or desktop/src/ that | ||
| # is not test-only: any such change requires a CHANGELOG.md edit or a new | ||
| # changelog.d/ fragment in the same PR. |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Align the changelog-rule description with the configured path scope. The rule matches every non-test path under tinyagentos/** and desktop/src/**, not only code files. tests/test_doc_gate.py lines 516-520 confirm that a tinyagentos/README.md edit triggers it.
docs/doc-gate.toml#L95-L97: replace “code change” with “non-test change,” or narrow the rule implementation to code files.CONTRIBUTING.md#L273-L276: describe the same non-test path scope.CHANGELOG.md#L17-L19: describe the same non-test path scope.
📍 Affects 3 files
docs/doc-gate.toml#L95-L97(this comment)CONTRIBUTING.md#L273-L276CHANGELOG.md#L17-L19
🤖 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 `@docs/doc-gate.toml` around lines 95 - 97, Align the changelog-rule
documentation with its configured non-test path scope: update docs/doc-gate.toml
lines 95-97, CONTRIBUTING.md lines 273-276, and CHANGELOG.md lines 17-19 to
describe every non-test change under tinyagentos/** or desktop/src/**, rather
than only code changes. No rule implementation change is needed.
|
nemotron-super review VERDICT: No blocking issues found. Automated first-pass review by the nemotron-super lane. The lead still reviews before merge. |
![kilo-code-bot[bot]](https://avatars.githubusercontent.com/in/2193792?s=60&v=4){kind=small}
| """Return (hash, author_name, message_body) for each commit in the range.""" | ||
| out = _run_git(["log", f"{base_ref}..HEAD", "--format=%H%x1f%an%x1f%B%x1f"]) | ||
| commits: list[tuple[str, str, str]] = [] | ||
| for block in out.split("\x1f"): |
There was a problem hiding this comment.
CRITICAL: _git_commits_with_messages always returns an empty list due to double-split parsing bug
out.split("\x1f") already separates the git log output into individual segments (hash, author, body), so the inner block.split("\x1f", 1) on line 326 can never produce more than one element. The len(parts) < 2 guard therefore skips every block, and the function always returns []. This breaks trailer detection in CI because main() derives commit_messages from this empty list, so evaluate_rules never sees a valid Docs-Reviewed: trailer and the gate fails regardless of whether the trailer is present.
Reply with @kilocode-bot fix it to have Kilo Code address this issue.
Code Review SummaryStatus: No Issues Found | Recommendation: Merge Files Reviewed (3 files)
Previous Review Summary (commit 451c5df)Current summary above is authoritative. Previous snapshots are kept for context only. Previous review (commit 451c5df)Status: 1 Issue Found | Recommendation: Address before merge Overview
Issue Details (click to expand)CRITICAL
Files Reviewed (5 files)
Reviewed by step-3.7-flash · Input: 83.9K · Output: 23.4K · Cached: 181.8K |
|
Red is REAL, not a flake: doc-gate's contributor-skill rule fired because this PR changes contribution rules (scripts/check_doc_gate.py + CONTRIBUTING.md) without an accompanying .md doc update - docs/doc-gate.toml does not satisfy it. Fix-forward requirements before this can gate:
Card tsk-3kmhmy stays with the lane; comment here when repushed. |
|
Lead review (this PR is queued LEAD-COMPLETE; the lane has been silent since the demands were posted). The trailer-audit half is inert. Proven against this branch, feeding the function exactly what that format produces: The four tests at Fix: parse the log output as records (split on the record terminator, then split each record into hash/author/body), and add one test that drives The rest of the change is sound and composes with the #2369 expansion already on dev: the changelog broadening to Still outstanding besides the above: the |
…ment the expanded rules Lead completion of the queued work on this branch. _git_commits_with_messages used the same separator for fields and records, so splitting the log output produced a flat list and every block failed the len(parts) < 2 guard: _log_trailer_usage never received a commit and no override was ever logged. The four existing tests passed because they hand-build the tuples and never exercise the producer. Records are now terminated with %x1e and fields separated with %x1f. Verified against real history, not only a fixture: over a range containing genuine Docs-Reviewed commits the gate now prints doc-gate: trailer override used in 58d035b by jaylfc: release version bump... Also updates the contributor skill's doc-gate table, which was stale before this PR and wrong after it: every rule, its trigger mode, and the test-path exemption are now listed, plus what the trailer override costs you.
|
Lead-completed in dc70162. The inert half is fixed and proven on real history, not a fixture. Records are now terminated with After the fix, run against a real range containing genuine Three new tests cover the producer seam the existing four could not reach. The Gate and invariants both clean locally, 65 doc-gate tests green. Merging on CI green. Worth knowing for anyone editing |
Both exemption cases used tests/test_agents.py, which matches neither rule's when_changed glob, so they passed whether or not _is_test_path existed. They now use paths inside the globs (tinyagentos/routes/test_agents.py, and both a Python and a co-located frontend test for the broad rule). Proven sensitive: neutering _is_test_path turns both red, which it could not do before. Also corrects two descriptions that did not match the config: the broad rule covers every NON-test path under those trees, not only code (the suite pins tinyagentos/README.md as a trigger), and the agent-manual rule is satisfied by docs/agent-coordination.md as well as an agent-manual page. Fenced example carries a language identifier (MD040).
|
Second lead pass: the remaining bot findings on this PR are addressed in 45928ea, and dev is merged in (beta.48 + LoRA Studio had landed since the branch was cut). The test-exemption tests were inert too, which CodeRabbit called and I confirmed: both used Also folded, all verified against the config rather than taken on faith:
Both bots had independently flagged the inert doc-gate clean, invariants clean, 65 doc-gate tests green on the merged branch. Merging on CI green. |
There was a problem hiding this comment.
Actionable comments posted: 1
🔇 Additional comments (5)
.claude/skills/taos-development-skill/SKILL.md (2)
431-434: 🎯 Functional Correctness
⚠️ Unverified finding
Sandbox verification was unavailable.Verify rename and copy status coverage for “any change” rules.
In
scripts/check_doc_gate.py, Lines 226-281,evaluate_rulesadds onlyA,D, andMwhenon_modifyis enabled torule_structural_paths. This guidance promises coverage for any non-test change. If the Git diff producer passes statuses such asR100orC100, renames or copies undertinyagentos/ordesktop/src/can bypass these rules. Confirm that the producer filters or normalizes those statuses. If it does not, handle them in the evaluator and add regression tests.Verification script
Also applies to: 443-452
461-464: LGTM!tests/test_doc_gate.py (3)
459-466: LGTM!
516-523: LGTM!
590-608: LGTM!
🤖 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 `@tests/test_doc_gate.py`:
- Around line 573-588: Update TestCommitsWithMessagesParsing to document the
%x1e record terminator, capture the args passed through the monkeypatched
_run_git in _parse, and assert that _git_commits_with_messages requests the
expected Git log format string, ensuring separator changes fail the contract
test.
🪄 Autofix
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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 676f45b5-e50a-42ed-92c4-e1bcbc1b095b
📒 Files selected for processing (5)
.claude/skills/taos-development-skill/SKILL.mdCONTRIBUTING.mddocs/doc-gate.tomlscripts/check_doc_gate.pytests/test_doc_gate.py
🚧 Files skipped from review as they are similar to previous changes (3)
- scripts/check_doc_gate.py
- docs/doc-gate.toml
- CONTRIBUTING.md
| class TestCommitsWithMessagesParsing: | ||
| """The producer half of the trailer audit. | ||
|
|
||
| The tests above hand-build the tuples, so they pass whether or not | ||
| anything can actually produce them. These drive the parser with the exact | ||
| bytes `git log --format=%H%x1f%an%x1f%B%x1f` emits. | ||
| """ | ||
|
|
||
| LOG_FORMAT_OUTPUT = ( | ||
| "abc1234567890\x1fJohn Doe\x1ffix: something\n\nDocs-Reviewed: internal refactor\n\x1e" | ||
| "\ndef4567890123\x1fJane Roe\x1ffeat: another thing\n\x1e" | ||
| ) | ||
|
|
||
| def _parse(self, monkeypatch, out): | ||
| monkeypatch.setattr(dg, "_run_git", lambda args: out) | ||
| return dg._git_commits_with_messages("origin/dev") |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -eu
printf '%s\n' '--- target test section ---'
sed -n '560,625p' tests/test_doc_gate.py
printf '%s\n' '--- parser implementation ---'
sed -n '300,350p' scripts/check_doc_gate.py
printf '%s\n' '--- related definitions and call sites ---'
rg -n -C 4 '_git_commits_with_messages|_run_git|%x1f|%x1e' scripts/check_doc_gate.py tests/test_doc_gate.pyRepository: jaylfc/taOS
Length of output: 1908
🌐 Web query:
site:github.com/jaylfc/taOS "class TestCommitsWithMessagesParsing" "_git_commits_with_messages"
💡 Result:
The terms TestCommitsWithMessagesParsing and _git_commits_with_messages are related to internal testing and development utilities within the taOS project codebase (an open-source repository maintained by jaylfc) [1][2][3]. These identifiers are associated with Python-based test suites or helper functions designed to validate Git operations, such as branch switching, repository management, and commit log parsing [1]. Specifically, the codebase includes logic to handle Git command execution safely, often involving parsing Git output or verifying branch/reference names to prevent security issues like flag injection [2][3]. While they do not appear to be part of a standard public API, they function as internal implementation details used in the project's update and version control automation [1][3].
Citations:
🏁 Script executed:
#!/bin/bash
set -eu
for sha in 2ba8eec38daffb86f3ca1230ad3ec39a789d6744 a1ea43beef488a13b0bb3f54664ddac69a0a71b4 a79c8ed2ac52adb958b38f33576d741d5762a54f; do
echo "--- $sha: candidate parser references ---"
curl -fsSL "https://raw.githubusercontent.com/jaylfc/taOS/$sha/scripts/check_doc_gate.py" |
rg -n -C 8 '_git_commits_with_messages|_run_git|%x1f|%x1e' || true
echo "--- $sha: candidate test references ---"
curl -fsSL "https://raw.githubusercontent.com/jaylfc/taOS/$sha/tests/test_doc_gate.py" |
rg -n -C 10 'TestCommitsWithMessagesParsing|LOG_FORMAT_OUTPUT|git log --format|seen_args|_parse' || true
doneRepository: jaylfc/taOS
Length of output: 1908
🌐 Web query:
site:github.com/jaylfc/taOS "origin/dev..HEAD" "--format=%H%x1f%an%x1f%B%x1e"
💡 Result:
The command git log origin/dev..HEAD --format=%H%x1f%an%x1f%B%x1e is used to display a custom-formatted log of commits that exist in the current branch (HEAD) but not in the origin/dev branch. Breakdown of the command components: 1. origin/dev..HEAD: This range selection tells Git to show only the commits reachable from HEAD that are not reachable from origin/dev. It is commonly used to see the new commits added to the current feature branch since it was last synced with the dev branch. 2. --format=%H%x1f%an%x1f%B%x1e: This specifies a custom output format for each commit, using placeholders and escape sequences to create a machine-readable, delimited list: - %H: The commit hash (full length). - %x1f: A horizontal tab or ASCII Unit Separator (hex 1F), used to delimit the hash, author name, and body. - %an: The author name. - %B: The raw commit body (including title and description). - %x1e: An ASCII Record Separator (hex 1E), used to denote the end of each commit entry. This specific format is often used in scripts or tools that need to parse commit data reliably, as the rare control characters (1F and 1E) are unlikely to appear naturally in commit messages, making them ideal for splitting entries and fields.
Citations:
- 1: a1ea43b
- 2: 2ba8eec
- 3: a79c8ed
- 4: docs(skill): add taos-development-skill (coding standards for contributing agents) #1763
- 5: https://github.com/jaylfc/taOS
Keep the parser contract test synchronized
Change the docstring terminator from %x1f to %x1e. Capture _run_git arguments and assert the expected Git format string so separator regressions fail the test.
🤖 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 `@tests/test_doc_gate.py` around lines 573 - 588, Update
TestCommitsWithMessagesParsing to document the %x1e record terminator, capture
the args passed through the monkeypatched _run_git in _parse, and assert that
_git_commits_with_messages requests the expected Git log format string, ensuring
separator changes fail the contract test.
1 participant
CARD TITLE (intent, not commit subject): Doc-gate expansion: CHANGELOG + modifications + broader agent-doc coverage
Autonomous build of board card tsk-3kmhmy.
Files:
CHANGELOG.md | 21 +++++++
CONTRIBUTING.md | 48 +++++++++++++---
docs/doc-gate.toml | 34 ++++++++----
scripts/check_doc_gate.py | 40 +++++++++++++-
tests/test_doc_gate.py | 138 ++++++++++++++++++++++++++++++++++++++++++++++
5 files changed, 262 insertions(+), 19 deletions(-)
Summary by CodeRabbit
Documentation
CI and Validation
Docs-Reviewedoverride.Tests