Add @total-typescript/shoehorn for type-safe test fixtures#28
Conversation
Install shoehorn in API and shared test packages and add the migrate-to-shoehorn agent skill for future test migrations. Co-authored-by: Cursor <cursoragent@cursor.com>
|
Warning Review limit reached
Next review available in: 53 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughThis PR adds a new documentation skill file describing migration from ChangesMigrate-to-shoehorn Skill Addition
Estimated code review effort🎯 1 (Trivial) | ⏱️ ~3 minutes Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 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 |
Use fromAny and fromPartial for Prisma, IdentityService, and request mocks in identity and controller tests. Co-authored-by: Cursor <cursoragent@cursor.com>
![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
🧹 Nitpick comments (1)
.agents/skills/migrate-to-shoehorn/SKILL.md (1)
112-118: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winUse
pnpminstead ofnpmand broaden test file patterns.The workflow at Line 113 uses
npm i@total-typescript/shoehorn, but the presence of `pnpm-lock.yaml` indicates this is a pnpm monorepo. Use `pnpm add -D `@total-typescript/shoehorn(or workspace-appropriate command) instead.Additionally, the grep pattern at Line 114 only matches
.test.tsand.spec.ts, missing.test.tsx,.spec.tsx, and other test variants. The regex" as [A-Z]"also missesas unknown as Typepatterns and may produce false positives. Consider:grep -rE " as (unknown )?[A-Z]" --include="*.test.*" --include="*.spec.*"🤖 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 @.agents/skills/migrate-to-shoehorn/SKILL.md around lines 112 - 118, The install and search workflow in the migrate-to-shoehorn steps needs to be updated for a pnpm monorepo and broader test coverage. In the SKILL.md migration instructions, replace the npm-based dependency install with the appropriate pnpm add command for `@total-typescript/shoehorn`, and update the test-file search step to match more variants than just *.test.ts and *.spec.ts. Also revise the grep pattern used in that step so it catches both direct and double assertions like as Type and as unknown as Type while avoiding narrow false negatives; use the existing migrate-to-shoehorn workflow bullets as the place to adjust these commands.
🤖 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 @.agents/skills/migrate-to-shoehorn/SKILL.md:
- Around line 1-118: The `migrate-to-shoehorn` lock entry has a `skillPath` that
does not match the actual `SKILL.md` location, so update the `skills-lock.json`
entry to point to the `.agents/skills/migrate-to-shoehorn/SKILL.md` path; use
the `migrate-to-shoehorn` skill name in `skills-lock.json` to find the affected
record and keep the path consistent with the documented skill file location.
---
Nitpick comments:
In @.agents/skills/migrate-to-shoehorn/SKILL.md:
- Around line 112-118: The install and search workflow in the
migrate-to-shoehorn steps needs to be updated for a pnpm monorepo and broader
test coverage. In the SKILL.md migration instructions, replace the npm-based
dependency install with the appropriate pnpm add command for
`@total-typescript/shoehorn`, and update the test-file search step to match more
variants than just *.test.ts and *.spec.ts. Also revise the grep pattern used in
that step so it catches both direct and double assertions like as Type and as
unknown as Type while avoiding narrow false negatives; use the existing
migrate-to-shoehorn workflow bullets as the place to adjust these commands.
🪄 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: b564ea99-9fd4-48c9-b529-04a43c416587
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (4)
.agents/skills/migrate-to-shoehorn/SKILL.mdapps/api/package.jsonpackages/shared/package.jsonskills-lock.json
| --- | ||
| name: migrate-to-shoehorn | ||
| description: Migrate test files from `as` type assertions to @total-typescript/shoehorn. Use when user mentions shoehorn, wants to replace `as` in tests, or needs partial test data. | ||
| --- | ||
|
|
||
| # Migrate to Shoehorn | ||
|
|
||
| ## Why shoehorn? | ||
|
|
||
| `shoehorn` lets you pass partial data in tests while keeping TypeScript happy. It replaces `as` assertions with type-safe alternatives. | ||
|
|
||
| **Test code only.** Never use shoehorn in production code. | ||
|
|
||
| Problems with `as` in tests: | ||
|
|
||
| - Trained not to use it | ||
| - Must manually specify target type | ||
| - Double-as (`as unknown as Type`) for intentionally wrong data | ||
|
|
||
| ## Install | ||
|
|
||
| ```bash | ||
| npm i @total-typescript/shoehorn | ||
| ``` | ||
|
|
||
| ## Migration patterns | ||
|
|
||
| ### Large objects with few needed properties | ||
|
|
||
| Before: | ||
|
|
||
| ```ts | ||
| type Request = { | ||
| body: { id: string }; | ||
| headers: Record<string, string>; | ||
| cookies: Record<string, string>; | ||
| // ...20 more properties | ||
| }; | ||
|
|
||
| it("gets user by id", () => { | ||
| // Only care about body.id but must fake entire Request | ||
| getUser({ | ||
| body: { id: "123" }, | ||
| headers: {}, | ||
| cookies: {}, | ||
| // ...fake all 20 properties | ||
| }); | ||
| }); | ||
| ``` | ||
|
|
||
| After: | ||
|
|
||
| ```ts | ||
| import { fromPartial } from "@total-typescript/shoehorn"; | ||
|
|
||
| it("gets user by id", () => { | ||
| getUser( | ||
| fromPartial({ | ||
| body: { id: "123" }, | ||
| }), | ||
| ); | ||
| }); | ||
| ``` | ||
|
|
||
| ### `as Type` → `fromPartial()` | ||
|
|
||
| Before: | ||
|
|
||
| ```ts | ||
| getUser({ body: { id: "123" } } as Request); | ||
| ``` | ||
|
|
||
| After: | ||
|
|
||
| ```ts | ||
| import { fromPartial } from "@total-typescript/shoehorn"; | ||
|
|
||
| getUser(fromPartial({ body: { id: "123" } })); | ||
| ``` | ||
|
|
||
| ### `as unknown as Type` → `fromAny()` | ||
|
|
||
| Before: | ||
|
|
||
| ```ts | ||
| getUser({ body: { id: 123 } } as unknown as Request); // wrong type on purpose | ||
| ``` | ||
|
|
||
| After: | ||
|
|
||
| ```ts | ||
| import { fromAny } from "@total-typescript/shoehorn"; | ||
|
|
||
| getUser(fromAny({ body: { id: 123 } })); | ||
| ``` | ||
|
|
||
| ## When to use each | ||
|
|
||
| | Function | Use case | | ||
| | --------------- | -------------------------------------------------- | | ||
| | `fromPartial()` | Pass partial data that still type-checks | | ||
| | `fromAny()` | Pass intentionally wrong data (keeps autocomplete) | | ||
| | `fromExact()` | Force full object (swap with fromPartial later) | | ||
|
|
||
| ## Workflow | ||
|
|
||
| 1. **Gather requirements** - ask user: | ||
| - What test files have `as` assertions causing problems? | ||
| - Are they dealing with large objects where only some properties matter? | ||
| - Do they need to pass intentionally wrong data for error testing? | ||
|
|
||
| 2. **Install and migrate**: | ||
| - [ ] Install: `npm i @total-typescript/shoehorn` | ||
| - [ ] Find test files with `as` assertions: `grep -r " as [A-Z]" --include="*.test.ts" --include="*.spec.ts"` | ||
| - [ ] Replace `as Type` with `fromPartial()` | ||
| - [ ] Replace `as unknown as Type` with `fromAny()` | ||
| - [ ] Add imports from `@total-typescript/shoehorn` | ||
| - [ ] Run type check to verify |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🔴 Critical
Fix skillPath mismatch in skills-lock.json.
The skills-lock.json entry for migrate-to-shoehorn declares skillPath as "skills/misc/migrate-to-shoehorn/SKILL.md", but this file is actually located at .agents/skills/migrate-to-shoehorn/SKILL.md. This path mismatch means the lock entry does not correctly resolve to the skill documentation. Update the skillPath in skills-lock.json to match the actual file location.
🧰 Tools
🪛 SkillSpector (2.3.7)
[warning] 99: [MP2] Context Window Stuffing: Skill attempts to fill the context window with filler content, displacing legitimate instructions and safety constraints. This can degrade agent performance or bypass safety boundaries.
Remediation: Implement context-window management that detects and rejects padding or stuffing attempts. Prioritize system instructions over user-injected content.
(Memory Poisoning (MP2))
🤖 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 @.agents/skills/migrate-to-shoehorn/SKILL.md around lines 1 - 118, The
`migrate-to-shoehorn` lock entry has a `skillPath` that does not match the
actual `SKILL.md` location, so update the `skills-lock.json` entry to point to
the `.agents/skills/migrate-to-shoehorn/SKILL.md` path; use the
`migrate-to-shoehorn` skill name in `skills-lock.json` to find the affected
record and keep the path consistent with the documented skill file location.
1 participant
Summary
@total-typescript/shoehornin@cortex/apiand@cortex/sharedtest packagesastype assertions were found in test files; this prepares the repo for type-safe partial fixtures going forwardTest plan
@cortex/sharedtype-check passes@cortex/sharedtests pass (21/21)Made with Cursor
Summary by CodeRabbit