docs: align agent instructions and coverage config - #138
Conversation
WalkthroughDocumentation and configuration restructuring: agent documentation path moved from Changes
Estimated code review effort🎯 1 (Trivial) | ⏱️ ~5 minutes These changes are primarily documentation and configuration updates with no logic modifications. Verification focuses on reference consistency and path accuracy across files. Possibly related PRs
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ 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
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
README.md(2 hunks)agents.md(1 hunks)jest.config.js(1 hunks)
🔇 Additional comments (6)
agents.md (1)
37-81: Well-structured deduplication guidelines.The new section provides clear, actionable guidance on eliminating code duplication with practical steps, metrics, and anti-patterns. The structure (purpose, application scenarios, rules, steps, metrics, anti-patterns, tags, examples) makes it easy to reference and apply.
README.md (5)
14-14: Agent documentation reference correctly updated.The change from
.codex/agents/AGENTS.mdtoagents.mdaccurately reflects the actual file location in the repository.
21-21: Directory documentation corrected.Adding the
issues/directory and removing the non-existentdocs/directory accurately reflects the repository structure.
25-26: Script and VSCode directories documented.The documentation now includes the
script/directory (which aligns with the Jest coverage path correction) and thevscode/directory, providing a more complete view of the repository structure.
156-156: Agent documentation reference updated consistently.The reference is correctly updated from
AGENTS.mdtoagents.mdto match the actual filename.
151-151: The review comment is incorrect—the sync mechanism referenced in the README is user-level, not repo-level.The repository does not contain a
.codex/setup-agents.shor any repo-level script that syncsagents.mdto~/.codex/agents.md. The README accurately references~/.codex/scripts/sync-agents.sh(lines 122, 125, 128) as user-initiated tools for end-users to optionally sync the agents documentation into their home directory. These are not part of the repository setup; they're documented features for users to run manually. The claim in line 151 that agents.md syncs "after setup" refers to user action, not automatic repository setup. There is nothing in the codebase to verify on this point.Likely an incorrect or invalid review comment.
| testEnvironment: 'node', | ||
| testMatch: ['**/test/**/*.test.js', '**/test/**/*.spec.js'], | ||
| collectCoverageFrom: ['scripts/**/*.js', '!scripts/**/*.test.js', '!scripts/**/*.spec.js'], | ||
| collectCoverageFrom: ['script/**/*.js', '!script/**/*.test.js', '!script/**/*.spec.js'], |
There was a problem hiding this comment.
🧩 Analysis chain
Coverage path correctly updated to match directory structure.
The change from scripts/ to script/ aligns with the documented directory structure in README.md (line 25).
Verify that the script/ directory exists and contains JavaScript files:
🏁 Script executed:
#!/bin/bash
# Description: Verify script/ directory exists and contains .js files that should be tracked for coverage
# Check if script/ directory exists
if [ -d "script" ]; then
echo "✓ script/ directory exists"
# List all .js files in script/ (excluding test files)
echo ""
echo "JavaScript files in script/:"
fd -e js -E '*.test.js' -E '*.spec.js' . script/
else
echo "✗ script/ directory not found"
exit 1
fi
# Check if scripts/ directory exists (old path)
if [ -d "scripts" ]; then
echo ""
echo "⚠ Warning: scripts/ directory still exists (should be removed or renamed)"
fiLength of output: 56
🏁 Script executed:
#!/bin/bash
# Comprehensive verification of directory structure and documentation
echo "=== Checking script/ directory contents ==="
if [ -d "script" ]; then
echo "✓ script/ exists"
find script -type f -name "*.js" | head -20
else
echo "✗ script/ not found"
fi
echo ""
echo "=== Checking scripts/ directory (old path) ==="
if [ -d "scripts" ]; then
echo "✓ scripts/ exists"
find scripts -type f -name "*.js" | head -20
else
echo "✗ scripts/ not found"
fi
echo ""
echo "=== Checking README.md line 25 context ==="
if [ -f "README.md" ]; then
sed -n '20,30p' README.md
else
echo "README.md not found"
fi
echo ""
echo "=== Current jest.config.js collectCoverageFrom ==="
grep -A 1 "collectCoverageFrom" jest.config.jsLength of output: 1571
jest.config.js coverage pattern is broken — script/ contains shell scripts, not JavaScript files.
The directory rename from scripts/ to script/ is correct per README.md documentation. However, the jest coverage pattern searches for *.js files while the script/ directory contains only shell scripts (.sh): export.sh, import.sh, commit_changes.sh, credentials.sh, brew-deps.sh, version.sh, update-libraries.sh. The coverage glob will match zero files.
Either remove the script/**/*.js pattern from collectCoverageFrom if shell scripts shouldn't be covered, or correct the configuration if JavaScript files should exist in this directory.
🤖 Prompt for AI Agents
In jest.config.js around line 4, the collectCoverageFrom glob 'script/**/*.js'
targets JavaScript files but the script/ folder contains only shell scripts, so
the pattern matches nothing; either remove the 'script/**/*.js' pattern from
collectCoverageFrom (if shell scripts shouldn't be covered) or replace it with
the correct glob for JS files in the intended directory (e.g., 'scripts/**/*.js'
if the directory should be scripts/), then run tests to confirm coverage is
collected as expected.
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
AGENTS.md(1 hunks)README.md(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- README.md
🧰 Additional context used
🪛 markdownlint-cli2 (0.18.1)
AGENTS.md
78-78: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
🔇 Additional comments (1)
AGENTS.md (1)
37-76: Documentation section well-structured and aligns with PR objective.The new "非冗長にして重複を排除する" section comprehensively documents the deduplication policy with clear guidance on purpose, applicable scenarios, rules, procedure, metrics, anti-patterns, and related tags. This directly fulfills the PR objective to document the policy in
AGENTS.mdfor easier reuse across the project.Placement after "Codex コマンド連携" and before "参考ドキュメント" is logical and maintains document flow.
| ``` | ||
| refactor:dedupe date range handling across billing/reporting | ||
| ``` |
There was a problem hiding this comment.
Specify language identifier for fenced code block.
The code block lacks a language identifier. Per Markdown best practices and markdownlint (MD040), add a language specifier (e.g., text, shell, or diff if the example follows Git diff format).
-```
+```text
refactor:dedupe date range handling across billing/reporting
-```
+```🧰 Tools
🪛 markdownlint-cli2 (0.18.1)
78-78: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
🤖 Prompt for AI Agents
In AGENTS.md around lines 78 to 80 the fenced code block lacks a language
identifier; update the opening fence to include a language (for example "text",
"diff", or "shell") so markdownlint MD040 is satisfied and syntax highlighting
is clearer (e.g., change ``` to ```text or ```diff depending on intent).
Summary
agents.mdfor easier reuseREADME.mdto reference the actualagents.mdfile, replace the non-existentdocs/directory description withissues/, and clarify where agent docs sync after setupscript/are tracked correctlyTesting
npm testSummary by CodeRabbit
Documentation
Chores