feat: strengthen Company Brain context bridge#32
Conversation
📝 WalkthroughWalkthroughThis PR enhances Company Brain account resolution and context formatting by exporting a reusable account-resolution function, refactoring the context resolver to use it, and extending context formatting to include aggregated follow-up records. Documentation and tests validate the new behavior. ChangesCompany Brain Account Resolution and Context Formatting
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related PRs
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 docstrings
🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
ESLint skipped: no ESLint configuration detected in root package.json. To enable, add Comment |
There was a problem hiding this comment.
🧹 Nitpick comments (1)
src/__tests__/company-brain-context-format.test.ts (1)
26-62: ⚡ Quick winConsider adding a test case for multiple matching accounts.
The stack outline specifies that the function "returns a resolved account object only when exactly one unambiguous candidate exists; returns null otherwise." The current tests cover successful single-match resolution and not-found scenarios, but don't verify the ambiguous case where multiple accounts could match.
✅ Suggested test case for multiple matches
Add a test block after line 62:
{ const resolved = resolveCompanyBrainAccountFromAccountsList( { accounts: [ { id: "acct_acme", name: "Acme Clinic", visibility_scope: "account" }, { id: "acct_acme_2", name: "Acme Clinic East", visibility_scope: "account" }, ], total: 2, }, { configuredAccountId: "acct_acme", search: "acct_acme", }, ); assert.equal(resolved, null, "Should return null when multiple candidates exist"); }🤖 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 `@src/__tests__/company-brain-context-format.test.ts` around lines 26 - 62, Add a test that verifies resolveCompanyBrainAccountFromAccountsList returns null when more than one account matches the search (ambiguous case): call resolveCompanyBrainAccountFromAccountsList with an accounts array containing two matching entries (e.g., "acct_acme" and "acct_acme_2" both with visibility_scope "account"), keep configuredAccountId/search set to "acct_acme", and assert the result is null to ensure the function treats multiple candidates as ambiguous.
🤖 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.
Nitpick comments:
In `@src/__tests__/company-brain-context-format.test.ts`:
- Around line 26-62: Add a test that verifies
resolveCompanyBrainAccountFromAccountsList returns null when more than one
account matches the search (ambiguous case): call
resolveCompanyBrainAccountFromAccountsList with an accounts array containing two
matching entries (e.g., "acct_acme" and "acct_acme_2" both with visibility_scope
"account"), keep configuredAccountId/search set to "acct_acme", and assert the
result is null to ensure the function treats multiple candidates as ambiguous.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 9248a979-e529-41a1-8291-75f884160dd1
⛔ Files ignored due to path filters (6)
dist/__tests__/company-brain-context-format.test.jsis excluded by!**/dist/**dist/__tests__/company-brain-context-format.test.js.mapis excluded by!**/dist/**,!**/*.mapdist/index.d.tsis excluded by!**/dist/**dist/index.d.ts.mapis excluded by!**/dist/**,!**/*.mapdist/index.jsis excluded by!**/dist/**dist/index.js.mapis excluded by!**/dist/**,!**/*.map
📒 Files selected for processing (4)
README.mddocs/company-brain-tools.mdsrc/__tests__/company-brain-context-format.test.tssrc/index.ts
Summary
<company-brain-context>blockValidation
npm testnpm run buildCloses #31
Refs electricsheephq/electric-sheep#2029
Summary by CodeRabbit
Documentation
New Features
Tests