fixes: doc sync, AGENTS.md#44
Conversation
|
Warning Rate limit exceeded
Your organization is not enrolled in usage-based pricing. Contact your admin to enable usage-based pricing to continue reviews beyond the rate limit, or try again in 39 minutes and 43 seconds. ⌛ 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. ℹ️ Review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (7)
WalkthroughRelease 0.7.3 includes fixes for LLM output handling without expected Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 2 | ❌ 3❌ Failed checks (2 warnings, 1 inconclusive)
✅ 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
package.json (1)
26-26:⚠️ Potential issue | 🟡 MinorStale version in
postinstallmessage.The
postinstallscript still printsaspens v0.7.2even thoughversionwas bumped to0.7.3. Users installing this release will see a misleading version string.🔧 Proposed fix
- "postinstall": "echo '\n 📌 aspens v0.7.2: Nested-project layouts (e.g. `.NET ~/apps/MyApp/MyApp/MyApp.csproj`) now yield first-class domains instead of a single wrapper domain. Pretty-printed `scan` also shows domains for C#/Java/Swift/PHP/Elixir projects.\n Re-run `aspens doc init` if a prior scan came up empty or under-detailed.\n\n 🌲 aspens is in active development — please keep it up to date.\n Run into issues? Let us know: https://github.com/aspenkit/aspens/issues\n'" + "postinstall": "echo '\n 📌 aspens v0.7.3: Fixes `doc sync` handling of LLM replies without `<file>` tags, and the Codex `AGENTS.md` transform now loads `CLAUDE.md` from disk when missing from canonical input.\n\n 🌲 aspens is in active development — please keep it up to date.\n Run into issues? Let us know: https://github.com/aspenkit/aspens/issues\n'"Or, longer-term, source this string from
package.jsonversionto keep them in sync automatically.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@package.json` at line 26, The postinstall message hardcodes "aspens v0.7.2" which is now stale after bumping package.json version to 0.7.3; update the string in the "postinstall" script inside package.json to reflect the current version (or replace the hardcoded version with a template that reads the package.json "version" value) so the printed version matches the package.json "version" field; target the "postinstall" script entry in package.json to make the change.
🧹 Nitpick comments (1)
src/commands/doc-sync.js (1)
312-316: Consider surfacing the "no<file>tags" signal even without--verbose.Without
--verbose, a non-empty unstructured LLM reply now silently maps toDocs are up to date. That's the intended fix, but it can also mask genuine model regressions. A non-fatalp.log.info(always shown) instead of a verbose-only warn would let users notice when the LLM is "talking" instead of emitting tags.- if (baseFiles.length === 0 && result.text.trim().length > 0 && !/<file\s+path=/i.test(result.text)) { - if (verbose) { - p.log.warn('LLM responded without <file> tags — treating as no updates needed.'); - } - } + const unstructuredReply = + baseFiles.length === 0 && + result.text.trim().length > 0 && + !/<file\s+path=/i.test(result.text); + if (unstructuredReply) { + p.log.info( + verbose + ? 'LLM responded without <file> tags — treating as no updates needed.' + : 'No file updates emitted by LLM (run with --verbose for details).' + ); + }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/commands/doc-sync.js` around lines 312 - 316, The code currently only warns when verbose is true if the LLM returned non-empty text without <file> tags (baseFiles.length === 0 && result.text.trim().length > 0 && !/<file\s+path=/i.test(result.text)) using p.log.warn inside an if (verbose) block; change this to always emit a non-fatal informational message so users see the signal: remove the verbose guard and call p.log.info (or p.log.info with the same or slightly clearer message) instead of p.log.warn, referencing the same condition and the symbols baseFiles, result.text, p.log.warn/p.log.info.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/lib/target-transform.js`:
- Around line 55-60: The code currently checks existsSync then calls
readFileSync which can still throw; replace that pattern by attempting to read
the file inside a try/catch: compute diskPath as join(repoPath,
sourceTarget.instructionsFile) and then wrap readFileSync(diskPath, 'utf8') in a
try/catch, on success set instructionsFile = { path:
sourceTarget.instructionsFile, content: <data> } and on failure leave
instructionsFile as null (or undefined) so the transform doesn't throw; remove
the redundant existsSync check (and its import if unused) and ensure the catch
swallows the error consistent with other helpers in src/lib/*.js.
---
Outside diff comments:
In `@package.json`:
- Line 26: The postinstall message hardcodes "aspens v0.7.2" which is now stale
after bumping package.json version to 0.7.3; update the string in the
"postinstall" script inside package.json to reflect the current version (or
replace the hardcoded version with a template that reads the package.json
"version" value) so the printed version matches the package.json "version"
field; target the "postinstall" script entry in package.json to make the change.
---
Nitpick comments:
In `@src/commands/doc-sync.js`:
- Around line 312-316: The code currently only warns when verbose is true if the
LLM returned non-empty text without <file> tags (baseFiles.length === 0 &&
result.text.trim().length > 0 && !/<file\s+path=/i.test(result.text)) using
p.log.warn inside an if (verbose) block; change this to always emit a non-fatal
informational message so users see the signal: remove the verbose guard and call
p.log.info (or p.log.info with the same or slightly clearer message) instead of
p.log.warn, referencing the same condition and the symbols baseFiles,
result.text, p.log.warn/p.log.info.
🪄 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: ac978311-5095-44a7-8083-7c93edc5b126
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (7)
CHANGELOG.mdREADME.mdpackage.jsonsrc/commands/doc-init.jssrc/commands/doc-sync.jssrc/lib/target-transform.jssrc/prompts/doc-sync.md
readFileSync in a try/catch
1 participant
What
Why
Closes #
How I tested
npm testpasses--dry-runoutput looks correct (if applicable)Checklist
Summary by CodeRabbit
Bug Fixes
doc syncnow handles incomplete LLM responses gracefully instead of failingAGENTS.mdnow automatically loads missing dependencies from disk, fixing incomplete outputs fromdoc init --strategy skip-existingDocumentation
aspensworkflows