Fix/test typescript errors#472
Conversation
📝 WalkthroughWalkthroughThis PR removes the entire MCP (Model Context Protocol) server package ( Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 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 |
Deploying with
|
| Status | Name | Latest Commit | Updated (UTC) |
|---|---|---|---|
| ✅ Deployment successful! View logs |
corates | 7907ad7 | Apr 07 2026, 09:36 PM |
![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
🧹 Nitpick comments (5)
packages/web/src/lib/__tests__/referenceParser.test.ts (2)
156-176: Consider usingPartial<NormalizedReference>instead of double assertion.The
as unknown as NormalizedReferencepattern bypasses type safety entirely. IfgetRefDisplayNameonly needsfirstAuthorandpublicationYear, consider updating its signature to acceptPartial<NormalizedReference>orPick<NormalizedReference, 'firstAuthor' | 'publicationYear'>. This would allow test fixtures to remain partial without unsafe casts.If changing the function signature isn't feasible, the current approach with the explanatory comment is acceptable.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/web/src/lib/__tests__/referenceParser.test.ts` around lines 156 - 176, Update the tests to avoid the unsafe "as unknown as NormalizedReference" cast by using proper partial types or adjusting the function signature: either change the test fixtures to be typed as Partial<NormalizedReference> (or Pick<NormalizedReference, 'firstAuthor' | 'publicationYear'>) when calling getRefDisplayName, or change getRefDisplayName's parameter type from NormalizedReference to Partial<NormalizedReference> (or the Pick variant) so partial objects are accepted without double assertions; reference getRefDisplayName and NormalizedReference when making the change.
6-12: Use the@/import alias per project conventions.The import should use the configured alias instead of a relative path.
As per coding guidelines: "Use import aliases from jsconfig.json, with
@/mapping to packages/web/src/"Suggested fix
-import { - parseRIS, - parseBibTeX, - parseReferences, - getRefDisplayName, - type NormalizedReference, -} from '../referenceParser.js'; +import { + parseRIS, + parseBibTeX, + parseReferences, + getRefDisplayName, + type NormalizedReference, +} from '@/lib/referenceParser.js';🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/web/src/lib/__tests__/referenceParser.test.ts` around lines 6 - 12, Update the import path in the test to use the project alias: replace the relative import of referenceParser.js with the "@/lib/referenceParser.js" alias so the named imports (parseRIS, parseBibTeX, parseReferences, getRefDisplayName, NormalizedReference) are imported via the configured jsconfig alias instead of a relative path..claude/CLAUDE.md (1)
153-153: Harden the clone guidance for reproducibility and safety.Line 153 is good, but the command should prefer pinning to a tag/commit and explicitly avoid executing code from
reference/clones. That reduces supply-chain and drift risk in agent workflows.Proposed doc tweak
-- **External library docs**: Prefer reading source from cloned upstream repos in the `reference/` directory over ad-hoc web fetches. If a library isn't there yet and you need its source, clone it in with `git clone --depth 20 https://github.com/owner/repo.git reference/repo`. This works better than MCP-style doc fetchers for deep questions. +- **External library docs**: Prefer reading source from cloned upstream repos in the `reference/` directory over ad-hoc web fetches. If a library isn't there yet, clone it with a pinned ref when possible (tag or commit), for example: +- `git clone --depth 1 --branch <tag-or-release> https://github.com/owner/repo.git reference/repo` +- Treat `reference/` as read-only documentation context; do not run code from cloned repos.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.claude/CLAUDE.md at line 153, Update the "External library docs" guidance (the paragraph that currently shows the git clone command) to recommend cloning with a pinned tag or commit (e.g., mention using --branch <tag> or cloning then checking out a specific commit) and add an explicit warning to never execute code from the reference/ clones (treat them as read-only artifacts) to reduce supply-chain and drift risk; ensure the revised text replaces the existing sentence that contains the git clone example and keeps the intent of preferring cloned upstream repos for deep questions.packages/web/src/hooks/__tests__/useAddStudies/deduplication.test.ts (1)
27-31: Consider resettingfixtureIdCounterin test setup.The counter is never reset between test runs, which could cause non-deterministic IDs across test files if tests depend on specific ID values. While current tests don't appear to rely on specific IDs, consider adding a
beforeEachreset for robustness.Optional: Reset counter between tests
+beforeEach(() => { + fixtureIdCounter = 0; +}); + let fixtureIdCounter = 0;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/web/src/hooks/__tests__/useAddStudies/deduplication.test.ts` around lines 27 - 31, Reset the shared fixtureIdCounter between tests to avoid non-deterministic IDs: add a test setup hook (e.g., a beforeEach in this test file) that sets fixtureIdCounter = 0 so nextId(prefix) generates deterministic IDs; reference the existing fixtureIdCounter and nextId helper to locate where to add the beforeEach reset.packages/web/src/lib/__tests__/checklist-domain.test.ts (1)
710-712: Remove unnecessaryas stringcast on enum value.
CHECKLIST_STATUS.IN_PROGRESSis already typed as the string literal'in-progress'. This cast serves no purpose and is inconsistent with how the same constant is used throughout the codebase without casting. The test fixture'screateChecklist()helper (line 27) demonstrates the correct pattern—no cast is needed. Remove theas stringcast to maintain type safety and consistency.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/web/src/lib/__tests__/checklist-domain.test.ts` around lines 710 - 712, Remove the unnecessary type cast on the enum value: delete the "as string" cast on CHECKLIST_STATUS.IN_PROGRESS in the test fixture where checklists are defined; use CHECKLIST_STATUS.IN_PROGRESS directly (matching the createChecklist() helper usage) to restore correct typing and consistency with the codebase.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.github/Contributing.md:
- Line 154: Change the mismatched singular/plural in the sentence "If asked to
create a plan file, agents will create them in `packages/docs/audits`" so the
subject and pronoun agree; either make the subject plural ("If asked to create
plan files, agents will create them in `packages/docs/audits`.") or make the
pronoun singular ("If asked to create a plan file, an agent will create it in
`packages/docs/audits`."). Update the sentence accordingly in Contributing.md.
In `@packages/web/src/api/__tests__/better-auth-store.test.ts`:
- Around line 382-384: Replace the direct assignment to window.location in the
test (the line using (global.window as { location: { origin: string }
}).location = { origin: 'http://localhost:5173' }) with the same
delete-then-assign pattern used elsewhere: delete (global.window as
any).location; then assign the new location object so the read-only DOM typing
is bypassed consistently; update any related teardown to restore location if
present.
In `@packages/web/src/lib/__tests__/form-errors.test.ts`:
- Line 10: The import uses a relative path; change it to the repo alias by
replacing "../form-errors.js" with the "@/lib/form-errors.js" alias so the test
imports handleFormError (handleFormErrorStrict) and createFormErrorState via the
configured `@/` mapping; update the import statement in form-errors.test.ts to use
"@/lib/form-errors.js" to comply with jsconfig.json aliasing rules.
In `@packages/web/src/primitives/__tests__/db.test.ts`:
- Around line 10-11: The test imports use relative paths—replace the two imports
of '../db.js' so they use the repo alias (@" mapping to packages/web/src/)
instead of relative imports; update both import lines that reference DbModule
and ProjectRow in db.test.ts to import from the aliased path (e.g.,
'@/primitives/db' or '@/primitives/db.js') so they follow the jsconfig.json `@/`
mapping and align with repo import policy.
---
Nitpick comments:
In @.claude/CLAUDE.md:
- Line 153: Update the "External library docs" guidance (the paragraph that
currently shows the git clone command) to recommend cloning with a pinned tag or
commit (e.g., mention using --branch <tag> or cloning then checking out a
specific commit) and add an explicit warning to never execute code from the
reference/ clones (treat them as read-only artifacts) to reduce supply-chain and
drift risk; ensure the revised text replaces the existing sentence that contains
the git clone example and keeps the intent of preferring cloned upstream repos
for deep questions.
In `@packages/web/src/hooks/__tests__/useAddStudies/deduplication.test.ts`:
- Around line 27-31: Reset the shared fixtureIdCounter between tests to avoid
non-deterministic IDs: add a test setup hook (e.g., a beforeEach in this test
file) that sets fixtureIdCounter = 0 so nextId(prefix) generates deterministic
IDs; reference the existing fixtureIdCounter and nextId helper to locate where
to add the beforeEach reset.
In `@packages/web/src/lib/__tests__/checklist-domain.test.ts`:
- Around line 710-712: Remove the unnecessary type cast on the enum value:
delete the "as string" cast on CHECKLIST_STATUS.IN_PROGRESS in the test fixture
where checklists are defined; use CHECKLIST_STATUS.IN_PROGRESS directly
(matching the createChecklist() helper usage) to restore correct typing and
consistency with the codebase.
In `@packages/web/src/lib/__tests__/referenceParser.test.ts`:
- Around line 156-176: Update the tests to avoid the unsafe "as unknown as
NormalizedReference" cast by using proper partial types or adjusting the
function signature: either change the test fixtures to be typed as
Partial<NormalizedReference> (or Pick<NormalizedReference, 'firstAuthor' |
'publicationYear'>) when calling getRefDisplayName, or change
getRefDisplayName's parameter type from NormalizedReference to
Partial<NormalizedReference> (or the Pick variant) so partial objects are
accepted without double assertions; reference getRefDisplayName and
NormalizedReference when making the change.
- Around line 6-12: Update the import path in the test to use the project alias:
replace the relative import of referenceParser.js with the
"@/lib/referenceParser.js" alias so the named imports (parseRIS, parseBibTeX,
parseReferences, getRefDisplayName, NormalizedReference) are imported via the
configured jsconfig alias instead of a relative path.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: d04a1f94-48e1-4090-9561-e6aad1abf09c
⛔ Files ignored due to path filters (2)
packages/docs/plans/yjs-awareness.mdis excluded by!packages/docs/plans/**pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (57)
.claude/CLAUDE.md.github/Contributing.md.github/copilot-instructions.md.github/dependabot.yml.mcp.json.mcp.md.vscode/mcp.jsonREADME.mdknip.config.tspackage.jsonpackages/docs/STATUS.mdpackages/mcp/.gitignorepackages/mcp/README.mdpackages/mcp/__tests__/better-auth.test.tspackages/mcp/__tests__/drizzle.test.tspackages/mcp/__tests__/icons.test.tspackages/mcp/__tests__/zag.test.tspackages/mcp/iconSearch.jspackages/mcp/package.jsonpackages/mcp/src/constants.tspackages/mcp/src/docs/d3/llms.txtpackages/mcp/src/docs/dexie/llms.txtpackages/mcp/src/docs/solidjs/llms.txtpackages/mcp/src/docs/tailwind/llms.txtpackages/mcp/src/docs/y-dexie/llms.txtpackages/mcp/src/docs/yjs/llms.txtpackages/mcp/src/scrape-icons.tspackages/mcp/src/server.tspackages/mcp/src/tools/better-auth.tspackages/mcp/src/tools/code-review.tspackages/mcp/src/tools/drizzle.tspackages/mcp/src/tools/icons.tspackages/mcp/src/tools/lint.tspackages/mcp/src/tools/local-docs.tspackages/mcp/src/tools/stripe.tspackages/mcp/src/types.tspackages/mcp/tsconfig.jsonpackages/mcp/vitest.config.tspackages/web/src/api/__tests__/better-auth-store.test.tspackages/web/src/components/checklist/AMSTAR2Checklist/__tests__/checklist-compare.test.tspackages/web/src/components/checklist/AMSTAR2Checklist/__tests__/checklist.test.tspackages/web/src/components/checklist/ROBINSIChecklist/__tests__/robins-scoring.test.tspackages/web/src/hooks/__tests__/useAddStudies/deduplication.test.tspackages/web/src/hooks/__tests__/useAddStudies/matching.test.tspackages/web/src/lib/__tests__/access.test.tspackages/web/src/lib/__tests__/checklist-domain.test.tspackages/web/src/lib/__tests__/entitlements.test.tspackages/web/src/lib/__tests__/form-errors.test.tspackages/web/src/lib/__tests__/pdfUtils.test.tspackages/web/src/lib/__tests__/pdfValidation.test.tspackages/web/src/lib/__tests__/referenceLookup.test.tspackages/web/src/lib/__tests__/referenceParser.test.tspackages/web/src/primitives/__tests__/avatarCache.test.tspackages/web/src/primitives/__tests__/db.test.tspackages/web/src/primitives/__tests__/pdfCache.test.tspackages/web/src/primitives/useProject/checklists/handlers/rob2.test.tspackages/web/src/primitives/useProject/checklists/handlers/robins-i.test.ts
💤 Files with no reviewable changes (33)
- README.md
- package.json
- packages/mcp/tsconfig.json
- .mcp.md
- .mcp.json
- .vscode/mcp.json
- .github/dependabot.yml
- packages/mcp/.gitignore
- packages/mcp/src/constants.ts
- packages/mcp/README.md
- packages/mcp/tests/drizzle.test.ts
- packages/mcp/tests/better-auth.test.ts
- packages/mcp/tests/icons.test.ts
- packages/mcp/src/types.ts
- packages/mcp/src/scrape-icons.ts
- packages/mcp/src/docs/yjs/llms.txt
- packages/mcp/iconSearch.js
- packages/mcp/package.json
- packages/mcp/src/tools/icons.ts
- packages/mcp/src/tools/better-auth.ts
- packages/mcp/vitest.config.ts
- packages/mcp/src/tools/lint.ts
- packages/mcp/src/server.ts
- packages/mcp/src/tools/local-docs.ts
- packages/mcp/src/docs/y-dexie/llms.txt
- packages/mcp/src/docs/d3/llms.txt
- packages/mcp/src/tools/stripe.ts
- packages/mcp/src/docs/tailwind/llms.txt
- packages/mcp/tests/zag.test.ts
- packages/mcp/src/tools/code-review.ts
- knip.config.ts
- packages/docs/STATUS.md
- packages/mcp/src/tools/drizzle.ts
📜 Review details
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Workers Builds: corates
🧰 Additional context used
📓 Path-based instructions (2)
**/*.{js,jsx,ts,tsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Never use emojis or unicode symbols anywhere in code, comments, documentation, plan files, or commit messages
For UI icons, use lucide-react library or SVGs only (never emojis)
Use import aliases from jsconfig.json, with
@/mapping to packages/web/src/Prefer modern ES6+ syntax and features
Prefer config files over hardcoding values
Code comments should explain why something is being done or provide context, not narrate what the code is saying
Keep files small, focused, and modular; extract large files into sub-modules or separate utilities
Ensure browser compatibility with Safari
Use TODO(agent): pattern for incomplete work or flags that need follow-up with brief description
Files:
packages/web/src/primitives/__tests__/pdfCache.test.tspackages/web/src/lib/__tests__/pdfValidation.test.tspackages/web/src/lib/__tests__/access.test.tspackages/web/src/lib/__tests__/form-errors.test.tspackages/web/src/hooks/__tests__/useAddStudies/matching.test.tspackages/web/src/lib/__tests__/referenceLookup.test.tspackages/web/src/lib/__tests__/checklist-domain.test.tspackages/web/src/primitives/useProject/checklists/handlers/rob2.test.tspackages/web/src/lib/__tests__/entitlements.test.tspackages/web/src/lib/__tests__/pdfUtils.test.tspackages/web/src/primitives/__tests__/db.test.tspackages/web/src/components/checklist/ROBINSIChecklist/__tests__/robins-scoring.test.tspackages/web/src/api/__tests__/better-auth-store.test.tspackages/web/src/components/checklist/AMSTAR2Checklist/__tests__/checklist.test.tspackages/web/src/components/checklist/AMSTAR2Checklist/__tests__/checklist-compare.test.tspackages/web/src/lib/__tests__/referenceParser.test.tspackages/web/src/primitives/__tests__/avatarCache.test.tspackages/web/src/primitives/useProject/checklists/handlers/robins-i.test.tspackages/web/src/hooks/__tests__/useAddStudies/deduplication.test.ts
packages/web/src/**/*.{js,jsx,ts,tsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Use shadcn/ui for UI components from
@/components/ui/Use TanStack Router with file-based routing via createFileRoute
Use TanStack Query for server state management with useQuery and useMutation
Use Recharts for data visualization in admin charts
Use Better-Auth for authentication and user management
Files:
packages/web/src/primitives/__tests__/pdfCache.test.tspackages/web/src/lib/__tests__/pdfValidation.test.tspackages/web/src/lib/__tests__/access.test.tspackages/web/src/lib/__tests__/form-errors.test.tspackages/web/src/hooks/__tests__/useAddStudies/matching.test.tspackages/web/src/lib/__tests__/referenceLookup.test.tspackages/web/src/lib/__tests__/checklist-domain.test.tspackages/web/src/primitives/useProject/checklists/handlers/rob2.test.tspackages/web/src/lib/__tests__/entitlements.test.tspackages/web/src/lib/__tests__/pdfUtils.test.tspackages/web/src/primitives/__tests__/db.test.tspackages/web/src/components/checklist/ROBINSIChecklist/__tests__/robins-scoring.test.tspackages/web/src/api/__tests__/better-auth-store.test.tspackages/web/src/components/checklist/AMSTAR2Checklist/__tests__/checklist.test.tspackages/web/src/components/checklist/AMSTAR2Checklist/__tests__/checklist-compare.test.tspackages/web/src/lib/__tests__/referenceParser.test.tspackages/web/src/primitives/__tests__/avatarCache.test.tspackages/web/src/primitives/useProject/checklists/handlers/robins-i.test.tspackages/web/src/hooks/__tests__/useAddStudies/deduplication.test.ts
🧠 Learnings (13)
📓 Common learnings
Learnt from: CR
URL:
File: .github/copilot-instructions.md:undefined-undefined
Timestamp: 2026-04-07T21:39:08.475Z
Learning: Maintain source of truth policy: Documentation is authoritative. If code conflicts with documentation, either fix the code or update the documentation
Learnt from: CR
URL:
File: .github/copilot-instructions.md:undefined-undefined
Timestamp: 2026-04-07T21:39:08.475Z
Learning: Read documentation in order: core instructions, STATUS.md, AGENTS.md, then relevant guides from packages/docs/guides/
Learnt from: CR
URL:
File: .github/copilot-instructions.md:undefined-undefined
Timestamp: 2026-04-07T21:39:08.475Z
Learning: Never bypass Drizzle for database access
Learnt from: CR
URL:
File: .github/copilot-instructions.md:undefined-undefined
Timestamp: 2026-04-07T21:39:08.475Z
Learning: Never prop-drill shared state; import Zustand stores directly instead
Learnt from: CR
URL:
File: .github/copilot-instructions.md:undefined-undefined
Timestamp: 2026-04-07T21:39:08.475Z
Learning: Group related components in subdirectories with barrel exports
📚 Learning: 2026-03-29T04:40:36.262Z
Learnt from: CR
Repo: InfinityBowman/corates PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2026-03-29T04:40:36.262Z
Learning: Applies to packages/{workers,shared}/**/*.{ts,tsx} : Use Better-Auth for authentication and user management
Applied to files:
.github/Contributing.md.claude/CLAUDE.mdpackages/web/src/api/__tests__/better-auth-store.test.ts.github/copilot-instructions.md
📚 Learning: 2026-03-29T04:40:36.262Z
Learnt from: CR
Repo: InfinityBowman/corates PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2026-03-29T04:40:36.262Z
Learning: Applies to packages/{workers,shared}/**/*.{ts,tsx} : Use Zod for schema and input validation on the backend
Applied to files:
.github/Contributing.md.claude/CLAUDE.md.github/copilot-instructions.md
📚 Learning: 2026-03-29T04:40:36.262Z
Learnt from: CR
Repo: InfinityBowman/corates PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2026-03-29T04:40:36.262Z
Learning: Applies to packages/web/src/**/*.{tsx,ts} : Use `useSyncExternalStore` for external store subscriptions (e.g., Yjs awareness)
Applied to files:
packages/web/src/lib/__tests__/access.test.tspackages/web/src/primitives/__tests__/db.test.tspackages/web/src/api/__tests__/better-auth-store.test.ts
📚 Learning: 2026-03-29T04:40:36.262Z
Learnt from: CR
Repo: InfinityBowman/corates PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2026-03-29T04:40:36.262Z
Learning: Applies to packages/web/src/routes/**/*.{ts,tsx} : Use TanStack Router with file-based routing using `createFileRoute`
Applied to files:
.claude/CLAUDE.md.github/copilot-instructions.md
📚 Learning: 2026-03-29T04:40:36.262Z
Learnt from: CR
Repo: InfinityBowman/corates PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2026-03-29T04:40:36.262Z
Learning: Applies to packages/web/src/**/*.{tsx,ts} : Use TanStack Query for server state management with `useQuery` and `useMutation`
Applied to files:
.claude/CLAUDE.md.github/copilot-instructions.md
📚 Learning: 2026-03-29T04:40:36.262Z
Learnt from: CR
Repo: InfinityBowman/corates PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2026-03-29T04:40:36.262Z
Learning: Cloudflare Pages is NOT used; only Cloudflare Workers
Applied to files:
.claude/CLAUDE.md.github/copilot-instructions.md
📚 Learning: 2026-03-29T04:40:36.262Z
Learnt from: CR
Repo: InfinityBowman/corates PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2026-03-29T04:40:36.262Z
Learning: Applies to packages/web/src/**/*.{tsx,ts} : Use shadcn/ui for UI components (Radix-based, located in `@/components/ui/`)
Applied to files:
.claude/CLAUDE.md.github/copilot-instructions.md
📚 Learning: 2026-03-29T04:40:36.262Z
Learnt from: CR
Repo: InfinityBowman/corates PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2026-03-29T04:40:36.262Z
Learning: Applies to packages/web/src/components/**/*.{tsx,ts} : Group related components in subdirectories with barrel exports
Applied to files:
.claude/CLAUDE.md.github/copilot-instructions.md
📚 Learning: 2026-03-29T04:40:36.262Z
Learnt from: CR
Repo: InfinityBowman/corates PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2026-03-29T04:40:36.262Z
Learning: Applies to packages/web/src/**/*.{tsx,ts} : Use `useId()` for unique IDs on form elements (radio buttons, checkboxes)
Applied to files:
packages/web/src/hooks/__tests__/useAddStudies/matching.test.ts
📚 Learning: 2026-01-17T00:25:03.945Z
Learnt from: CR
Repo: InfinityBowman/corates PR: 0
File: .github/instructions/api-routes.instructions.md:0-0
Timestamp: 2026-01-17T00:25:03.945Z
Learning: Applies to packages/workers/**/routes/**/*.{js,ts} : Create DB client from environment using `createDb(c.env.DB)` instead of creating DB connections directly
Applied to files:
packages/web/src/primitives/__tests__/db.test.ts
📚 Learning: 2026-03-29T04:40:36.262Z
Learnt from: CR
Repo: InfinityBowman/corates PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2026-03-29T04:40:36.262Z
Learning: Applies to packages/web/src/stores/**/*.{ts,tsx} : Use Zustand for client state management; store files should be located in `@/stores/`
Applied to files:
packages/web/src/api/__tests__/better-auth-store.test.ts
📚 Learning: 2026-03-29T04:40:36.262Z
Learnt from: CR
Repo: InfinityBowman/corates PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2026-03-29T04:40:36.262Z
Learning: Applies to packages/web/src/**/*.{ts,tsx,js,jsx} : Use import aliases from jsconfig.json; `@/` maps to `packages/web/src/`
Applied to files:
packages/web/src/lib/__tests__/referenceParser.test.ts
🪛 LanguageTool
.claude/CLAUDE.md
[style] ~153-~153: Consider a different adjective to strengthen your wording.
Context: ... better than MCP-style doc fetchers for deep questions. - TanStack documentation...
(DEEP_PROFOUND)
🔇 Additional comments (33)
packages/web/src/lib/__tests__/pdfValidation.test.ts (2)
8-17: Typed test helper update looks good.
MockFileOptionsplus the typedcreateMockFile(...): Filesignature makes the helper safer and clearer without adding complexity.
170-172: Good type-safe assertion guard in skipped magic-bytes test.Conditionally asserting
result.erroronly whenresult.validis false avoids unsafe access and keeps the expectation aligned with the result shape.packages/web/src/lib/__tests__/pdfUtils.test.ts (3)
32-41: LGTM!The TypeScript type annotation
(title: string): stringis correct and improves type safety for this test helper function.
153-162: LGTM!The
MockFileReaderclass now has properly typed properties and uses optional chaining (this.onerror?.()) for safe invocation. This is a clean approach for implementing a minimal mock that covers only the error handling path being tested.
164-164: Acceptable double-assertion for test mock.The
as unknown as typeof FileReaderpattern is a pragmatic TypeScript workaround for mocking browser APIs with incomplete implementations. Since the mock intentionally implements only the subset needed for error testing, and the originalFileReaderis properly restored at line 170, this is fine for test code.packages/web/src/lib/__tests__/referenceLookup.test.ts (2)
22-25: Type annotations on mock parser inputs look good.This keeps the mock strict and improves test type safety without changing behavior.
45-45: DOMParser test override is acceptable for TypeScript compatibility.The cast is clear in intent for a test-only global shim.
packages/web/src/lib/__tests__/access.test.ts (1)
9-17: Good test-only type relaxation with a single local alias.This keeps defensive fixture coverage (
Partial,null,undefined) explicit without scattering casts across the assertions.packages/web/src/lib/__tests__/entitlements.test.ts (1)
9-41: Nice consolidation of permissive test typings for entitlements helpers.The shared
PartialSubscriptionand typed local aliases make the fixture surface explicit and reduce repetitive casting noise in individual tests..github/copilot-instructions.md (1)
142-142: Looks good — clear and actionable doc-source guidance.The updated instruction is concise and aligns with the repo’s doc-first workflow.
packages/web/src/api/__tests__/better-auth-store.test.ts (6)
14-33: Good approach to typing the mocked auth client.The
MockedAuthClientinterface paired with theloadMockedAuthClient()helper cleanly encapsulates the type bridging between the generic-heavy real client and the narrower mock surface. This avoids type gymnastics scattered throughout test cases.
101-107: Clean class-based mock for BroadcastChannel.The class-based approach is clearer and more maintainable than function-based mocks for constructor APIs.
110-131: localStorage mock now fully implements Storage interface.Adding
lengthandkey()with proper typing ensures the mock is complete and type-safe.
242-244: Type casts for test assertions are appropriate.Using inline type assertions like
as { twoFactorRequired: boolean }for test results is a reasonable trade-off. The assertions immediately validate the expected shape, maintaining runtime safety.
596-607: Standard fetch mock pattern.The
vi.fn() as unknown as typeof fetchassignment and subsequentvi.mocked(global.fetch)usage is the idiomatic way to mockfetchin Vitest while maintaining type safety.
37-43: Mock correctly replicatesauthFetchbehavior.The mock uses
unknowninstead of the genericT, which is appropriate for testing as it accepts any data type while preserving the error-handling semantics.packages/web/src/primitives/useProject/checklists/handlers/robins-i.test.ts (1)
10-11: Type tightening is well-contained and test-safe.The explicit
handler/doctyping andY.Textcast points are consistent with the runtime assertions in these tests.Also applies to: 23-25, 36-38, 47-49, 60-63
packages/web/src/primitives/useProject/checklists/handlers/rob2.test.ts (1)
44-53: Good containment of dynamic typing in the test harness.The typed declarations and centralized cast strategy keep dynamic Yjs access scoped and readable without changing runtime behavior.
Also applies to: 69-74, 78-79, 105-106, 125-126
packages/web/src/primitives/__tests__/db.test.ts (1)
18-28: Nice fixture refactor forProjectRowvalidity and reuse.
projectFixturekeeps test records schema-correct (ydocincluded) and removes repeated inline setup across cases.Also applies to: 82-83, 93-96, 87-89, 120-124, 202-204, 249-249
packages/web/src/lib/__tests__/checklist-domain.test.ts (1)
161-161: Non-null assertions are appropriate for these test assertions.The functions
getFinalizedChecklistandfindReconciledChecklistreturnChecklist | nullper their implementations. The test fixtures guarantee non-null results in these happy-path scenarios, soresult!.idis the correct fix for TypeScript strict mode.Also applies to: 175-175, 455-455, 466-466
packages/web/src/primitives/__tests__/avatarCache.test.ts (1)
31-34: Correct pattern for testing defensive runtime behavior.The
as unknown as stringcast is the appropriate way to test thatgetCachedAvatarhandles invalid inputs gracefully at runtime, while still satisfying TypeScript's type checker.packages/web/src/primitives/__tests__/pdfCache.test.ts (1)
27-29: Non-null assertions follow prior null checks.The tests assert
expect(retrieved).not.toBeNull()before accessing properties, so the!assertions are safe and appropriate for TypeScript narrowing.Also applies to: 47-49
packages/web/src/hooks/__tests__/useAddStudies/matching.test.ts (1)
178-181: Non-null assertions followtoBeDefined()assertions.Each
result!.idaccess is preceded byexpect(result).toBeDefined(), making the non-null assertions safe. This is the standard pattern when TypeScript cannot infer narrowing from test framework assertions.Also applies to: 184-188, 199-203, 205-210, 231-234, 249-253, 259-263
packages/web/src/components/checklist/ROBINSIChecklist/__tests__/robins-scoring.test.ts (2)
10-17: Well-structured local type helpers.The
Judgementtype derivation fromJUDGEMENTSand the updatedans/answershelpers properly handle the nullable answer cases. The comment explains why the type is derived locally rather than imported.
861-884: Clean refactor of test fixtures with proper typing.The
score()helper encapsulates the boilerplateScoringResultfields, andas constonjudgementSourceensures the literal union type is preserved. This improves type safety without changing test semantics.packages/web/src/hooks/__tests__/useAddStudies/deduplication.test.ts (1)
17-43: Well-designed test wrapper for partial fixtures.The
PartialSourcesinterface andbuildDeduplicatedStudieswrapper elegantly solve the problem of requiredid/_idfields while keeping test fixtures minimal. The auto-generated IDs ensure uniqueness, and the type casts are safe after filling required fields.packages/web/src/components/checklist/AMSTAR2Checklist/__tests__/checklist-compare.test.ts (2)
26-61: Appropriate loose type wrappers for testing defensive behavior.The
LooseComparisoninterface and type-cast wrappers allow tests to verify null handling and partial input scenarios without fabricating full typed fixtures. The comment clearly explains the rationale.
279-282: Correct use ofas constfor literal union types.The
as constassertions ensureselectionsvalues are typed as'reviewer1' | 'reviewer2'rather thanstring, matching the expected type ofcreateReconciledChecklist.packages/web/src/components/checklist/AMSTAR2Checklist/__tests__/checklist.test.ts (5)
20-23: KeyedChecklist helper type enables dynamic question access.The intersection type
AMSTAR2Checklist & Record<string, AMSTAR2Question>correctly allows indexing by dynamic question keys while preserving the base type. The comment clearly explains the need.
28-28: Standard pattern for testing validation error paths.Using
as anyto bypass TypeScript for invalid input tests is the correct approach when verifying runtime validation behavior.Also applies to: 46-46, 64-64, 68-68
179-181: Type annotations improve clarity in test helper.The explicit
Record<string, string>parameter type andAMSTAR2Checklistreturn type make the helper's contract clear. TheKeyedChecklistcast inside enables dynamic property access.Also applies to: 195-195
214-222: Correct casts for testing error handling.Using
null as unknown as AMSTAR2Checklistetc. allows testing thatscoreChecklisthandles invalid inputs gracefully while satisfying TypeScript.
502-502: Non-null assertions appropriate for consolidation tests.The
getAnswers(checklist)!pattern is acceptable here since tests verify the checklist is valid andgetAnswersshould return non-null for valid input. The assertions confirm expected consolidated values.Also applies to: 511-511, 527-527, 538-538, 549-549, 560-560, 570-570, 580-580, 590-590
| # 2. Type "Open MCP Settings" | ||
| # 3. Find 'corates' and start it if it has not been found and started already | ||
| ``` | ||
| If asked to create a plan file, agents will create them in `packages/docs/audits`. |
There was a problem hiding this comment.
Fix singular/plural wording for clarity.
Line 154 says “a plan file” but then uses “them.” Use singular (“it”) or pluralize the first phrase.
Proposed wording fix
-If asked to create a plan file, agents will create them in `packages/docs/audits`.
+If asked to create a plan file, agents will create it in `packages/docs/audits`.📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| If asked to create a plan file, agents will create them in `packages/docs/audits`. | |
| If asked to create a plan file, agents will create it in `packages/docs/audits`. |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.github/Contributing.md at line 154, Change the mismatched singular/plural
in the sentence "If asked to create a plan file, agents will create them in
`packages/docs/audits`" so the subject and pronoun agree; either make the
subject plural ("If asked to create plan files, agents will create them in
`packages/docs/audits`.") or make the pronoun singular ("If asked to create a
plan file, an agent will create it in `packages/docs/audits`."). Update the
sentence accordingly in Contributing.md.
| (global.window as { location: { origin: string } }).location = { | ||
| origin: 'http://localhost:5173', | ||
| }; |
There was a problem hiding this comment.
Inconsistent window.location replacement pattern.
This block directly assigns location without deleting it first, unlike lines 277-282 in the Social Auth tests which use delete before assignment. Since window.location is read-only in the DOM type definitions, the delete-first pattern is more robust.
Proposed fix for consistency
+ delete (global.window as { location?: unknown }).location;
(global.window as { location: { origin: string } }).location = {
origin: 'http://localhost:5173',
};📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| (global.window as { location: { origin: string } }).location = { | |
| origin: 'http://localhost:5173', | |
| }; | |
| delete (global.window as { location?: unknown }).location; | |
| (global.window as { location: { origin: string } }).location = { | |
| origin: 'http://localhost:5173', | |
| }; |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/web/src/api/__tests__/better-auth-store.test.ts` around lines 382 -
384, Replace the direct assignment to window.location in the test (the line
using (global.window as { location: { origin: string } }).location = { origin:
'http://localhost:5173' }) with the same delete-then-assign pattern used
elsewhere: delete (global.window as any).location; then assign the new location
object so the read-only DOM typing is bypassed consistently; update any related
teardown to restore location if present.
|
|
||
| import { describe, it, expect, vi } from 'vitest'; | ||
| import { handleFormError, createFormErrorState } from '../form-errors.js'; | ||
| import { handleFormError as handleFormErrorStrict, createFormErrorState } from '../form-errors.js'; |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Use @/ alias instead of relative import
Line 10 uses a relative import (../form-errors.js), which conflicts with the repo alias rule. Please switch to the @/ path (for example, @/lib/form-errors.js) for consistency.
As per coding guidelines: "Use import aliases from jsconfig.json, with @/ mapping to packages/web/src/"
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/web/src/lib/__tests__/form-errors.test.ts` at line 10, The import
uses a relative path; change it to the repo alias by replacing
"../form-errors.js" with the "@/lib/form-errors.js" alias so the test imports
handleFormError (handleFormErrorStrict) and createFormErrorState via the
configured `@/` mapping; update the import statement in form-errors.test.ts to use
"@/lib/form-errors.js" to comply with jsconfig.json aliasing rules.
| import type * as DbModule from '../db.js'; | ||
| import type { ProjectRow } from '../db.js'; |
There was a problem hiding this comment.
Use @/ aliases instead of relative imports in this test.
Line [10]-Line [11] currently use relative imports, which conflicts with repo import policy.
Proposed fix
-import type * as DbModule from '../db.js';
-import type { ProjectRow } from '../db.js';
+import type * as DbModule from '@/primitives/db.js';
+import type { ProjectRow } from '@/primitives/db.js';As per coding guidelines **/*.{js,jsx,ts,tsx}: Use import aliases from jsconfig.json, with @/ mapping to packages/web/src/.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/web/src/primitives/__tests__/db.test.ts` around lines 10 - 11, The
test imports use relative paths—replace the two imports of '../db.js' so they
use the repo alias (@" mapping to packages/web/src/) instead of relative
imports; update both import lines that reference DbModule and ProjectRow in
db.test.ts to import from the aliased path (e.g., '@/primitives/db' or
'@/primitives/db.js') so they follow the jsconfig.json `@/` mapping and align with
repo import policy.
2 participants
Summary by CodeRabbit
Release Notes
Chores
reference/directory instead of MCP tools.Tests