feat(SUP-1469): add jarvos gbrain bridge module#17
Conversation
|
Warning Rate limit exceeded
You’ve run out of usage credits. Purchase more in the billing tab. ⌛ 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: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (14)
📝 WalkthroughWalkthroughA new Changes@jarvos/gbrain Module Implementation
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested labels
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 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 |
|
@codex review |
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 48a91be382
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
90099ef to
7324257
Compare
|
@codex review |
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 73242572c6
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
docs/architecture/jarvos-architecture.md (1)
1-9:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winStale
updatedfrontmatter date.The YAML frontmatter still reads
updated: 2026-02-19, but this file is modified in the current PR (2026-05-08).✏️ Proposed fix
-updated: 2026-02-19 +updated: 2026-05-08🤖 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/architecture/jarvos-architecture.md` around lines 1 - 9, Update the YAML frontmatter's updated field in jarvos-architecture.md: replace the stale `updated: 2026-02-19` value with the current PR date (e.g., `2026-05-08`) or ensure it is programmatically set to the commit/merge date; specifically edit the `updated` frontmatter key so it reflects the current modification date in the file header.README.md (1)
217-217:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winPhilosophy section still lists six layers after the "Structured knowledge" addition.
Line 217 reads: "Content, recall, worldview, behavior, execution, and runtime each have a single owner" — missing the newly added "Structured knowledge" layer. This contradicts line 34 ("seven layers") and the layer table.
✏️ Proposed fix
-- **Layers, not features.** Content, recall, worldview, behavior, execution, and runtime each have a single owner. Don't mix them. +- **Layers, not features.** Content, recall, worldview, structured knowledge, behavior, execution, and runtime each have a single owner. Don't mix them.🤖 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 `@README.md` at line 217, The Philosophy section's sentence "Content, recall, worldview, behavior, execution, and runtime each have a single owner" is missing the new "Structured knowledge" layer; update that sentence to list all seven layers including "Structured knowledge" (e.g., "Content, structured knowledge, recall, worldview, behavior, execution, and runtime each have a single owner") so the text matches the "seven layers" claim and the layer table; ensure punctuation and plural agreement remain correct after insertion.
🧹 Nitpick comments (1)
modules/jarvos-gbrain/scripts/jarvos-gbrain.js (1)
41-80: ⚡ Quick win
main()has no error boundary — manifest/eval parse failures produce raw stack traces instead of clean error messages
readJsonFilealready formats descriptive error messages (Could not read JSON file …), but without a top-level try-catch they surface as unhandled exceptions with Node.js stack traces rather than a clean stderr line +exitCode = 1.♻️ Proposed fix
function main() { - const command = process.argv[2] || 'help'; - const config = cliConfig(); - // ... all dispatch logic ... - usage(); - process.exitCode = 1; + try { + const command = process.argv[2] || 'help'; + const config = cliConfig(); + // ... all dispatch logic ... + usage(); + process.exitCode = 1; + } catch (err) { + process.stderr.write(`Error: ${err.message}\n`); + process.exitCode = 1; + } }🤖 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 `@modules/jarvos-gbrain/scripts/jarvos-gbrain.js` around lines 41 - 80, main() lacks a top-level error boundary so parsing/manifest errors (from helpers like readJsonFile, createImportPlan, importToBrain, syncBrain, runRetrievalEval, doctor) produce raw stack traces instead of a clean stderr message and exit code; wrap the entire body of main() in a try/catch (or add a try/catch around the existing command dispatch) that catches any Error, prints a concise message to stderr (e.g., console.error(error.message || String(error))), sets process.exitCode = 1, and returns so all failures surface as a single clean error line rather than an unhandled exception.
🤖 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 `@modules/jarvos-gbrain/src/index.js`:
- Around line 137-140: The yamlScalar function currently only escapes double
quotes, which breaks YAML when values contain backslashes or newlines; update
yamlScalar (and its local variable text) to escape backslashes first and then
replace special characters used in YAML double-quoted scalars (at minimum
backslash, double-quote, newline, carriage return, and tab) so newlines become
\n and backslashes become \\ (and similarly for \r and \t) before wrapping with
quotes; ensure you perform the replacements in the correct order (escape
backslashes before other escapes) to produce valid YAML frontmatter.
- Line 76: The gbrainBin value returned from resolveConfig isn't being
tilde-expanded, causing spawnSync in runCommand to get a literal "~" path;
update resolveConfig so the assignment to gbrainBin uses expandTilde (same as
other resolved paths), or ensure runCommand calls expandTilde on gbrainBin
before passing it to spawnSync; reference the gbrainBin constant in
resolveConfig and the runCommand/spawnSync invocation, and keep commandExists
behavior consistent by not relying on it to perform the expansion.
- Around line 261-273: The code currently pushes an entry into the imported
array before attempting the filesystem operations, so a failed
mkdirSync/writeFileSync leaves a false success and aborts importToBrain; change
the flow in the loop that handles planned/body so that you perform
fs.mkdirSync(path.dirname(planned.targetPath), { recursive: true }) and
fs.writeFileSync(planned.targetPath, body, 'utf8') first (wrapped in try/catch
or let errors be handled per-item), and only after those calls succeed compute
bytes (Buffer.byteLength(body,'utf8')) and push the object to imported
(including dryRun flag); ensure failures do not leave a pushed entry and are
handled/logged so the loop can continue processing remaining planned items.
In `@modules/README.md`:
- Around line 176-183: Summary: README docs show unqualified defaults for
JARVOS_GBRAIN_IMPORT_MANIFEST and JARVOS_GBRAIN_EVAL_QUESTIONS which contradict
the actual defaults resolved by the module code. Fix: update the README entries
for the env vars JARVOS_GBRAIN_IMPORT_MANIFEST and JARVOS_GBRAIN_EVAL_QUESTIONS
so they match the qualified default paths that the module's index uses (the code
that resolves defaults uses MODULE_ROOT with path.join), and ensure both READMEs
contain the same qualified defaults to avoid users setting broken relative
paths.
---
Outside diff comments:
In `@docs/architecture/jarvos-architecture.md`:
- Around line 1-9: Update the YAML frontmatter's updated field in
jarvos-architecture.md: replace the stale `updated: 2026-02-19` value with the
current PR date (e.g., `2026-05-08`) or ensure it is programmatically set to the
commit/merge date; specifically edit the `updated` frontmatter key so it
reflects the current modification date in the file header.
In `@README.md`:
- Line 217: The Philosophy section's sentence "Content, recall, worldview,
behavior, execution, and runtime each have a single owner" is missing the new
"Structured knowledge" layer; update that sentence to list all seven layers
including "Structured knowledge" (e.g., "Content, structured knowledge, recall,
worldview, behavior, execution, and runtime each have a single owner") so the
text matches the "seven layers" claim and the layer table; ensure punctuation
and plural agreement remain correct after insertion.
---
Nitpick comments:
In `@modules/jarvos-gbrain/scripts/jarvos-gbrain.js`:
- Around line 41-80: main() lacks a top-level error boundary so parsing/manifest
errors (from helpers like readJsonFile, createImportPlan, importToBrain,
syncBrain, runRetrievalEval, doctor) produce raw stack traces instead of a clean
stderr message and exit code; wrap the entire body of main() in a try/catch (or
add a try/catch around the existing command dispatch) that catches any Error,
prints a concise message to stderr (e.g., console.error(error.message ||
String(error))), sets process.exitCode = 1, and returns so all failures surface
as a single clean error line rather than an unhandled exception.
🪄 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
Run ID: 6bc055b4-51a3-46d8-9097-020d731dbd74
📒 Files selected for processing (14)
PUBLIC_BASELINE.mdREADME.mddocs/architecture/jarvos-architecture.mdmodules/README.mdmodules/jarvos-gbrain/README.mdmodules/jarvos-gbrain/config/curated-import.jsonmodules/jarvos-gbrain/config/eval-questions.jsonmodules/jarvos-gbrain/package.jsonmodules/jarvos-gbrain/scripts/jarvos-gbrain.jsmodules/jarvos-gbrain/src/index.jsmodules/jarvos-gbrain/test/gbrain.test.jsscripts/smoke-test.shstarter-kit/README.mdtests/modules-smoke-test.js
7324257 to
82eb68b
Compare
|
@coderabbitai review |
|
@codex review |
✅ Actions performedReview triggered.
|
82eb68b to
9c5ecb2
Compare
|
@codex review |
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 9c5ecb2216
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
9c5ecb2 to
f80474f
Compare
|
@coderabbitai review |
|
@codex review |
✅ Actions performedReview triggered.
|
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: f80474f5d5
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| } | ||
|
|
||
| if (command === 'eval') { | ||
| printJson(runRetrievalEval(config, { dryRun: hasFlag('--dry-run') })); |
There was a problem hiding this comment.
When gbrain search itself exits successfully but the expected evidence is missing, runRetrievalEval() returns ok: false without throwing; this branch still just prints the JSON and returns, so node scripts/jarvos-gbrain.js eval exits 0 and any CI/npm workflow using the retrieval eval will pass despite failed questions. I reproduced this with a fake gbrain that outputs unrelated text and the CLI returned status 0 while reporting "ok": false.
Useful? React with 👍 / 👎.
1 participant
Summary
@jarvos/gbrainas a fourth jarVOS module for curated Obsidian-to-GBrain import planning, generated page writes, sync wrappers, doctor checks, and retrieval eval scaffoldingVerification
node --test modules/jarvos-gbrain/test/*.test.jsnode tests/modules-smoke-test.jsbash scripts/smoke-test.shnpm testnpm testinmodules/jarvos-gbrainplan,import --dry-run,sync --dry-run,eval --dry-run,doctorNotes
Summary by CodeRabbit
Release Notes
New Features
Documentation