-
-
Notifications
You must be signed in to change notification settings - Fork 11
merge dev to main #193
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
merge dev to main #193
Conversation
WalkthroughUpdates a TODO checklist, adjusts the User model’s computed field signature to accept a context parameter, aligns a related typecheck condition, and removes query logging configuration from a sample client initialization. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related PRs
Poem
Tip 🔌 Remote MCP (Model Context Protocol) integration is now available!Pro plan users can now connect to remote MCP servers from the Integrations page. Connect with popular remote MCPs such as Notion and Linear to add more context to your reviews and chats. ✨ Finishing Touches
🧪 Generate unit tests
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
Status, Documentation and Community
|
|
Claude finished @ymc9's task —— View job Code Review AnalysisReviewing this dev-to-main merge PR. Here's my task list:
Review SummaryThis PR contains clean, focused changes that improve the codebase without introducing any significant issues. All changes appear to be routine maintenance and improvements. Changes Analysis✅ TODO.md (Line 80)
✅ schema.ts (Lines 83-85)
✅ typecheck.ts (Line 14)
✅ main.ts (Line 9)
Code Quality AssessmentStrengths:
Best Practices Followed:
Security & PerformanceSecurity: ✅ No security concerns identified
Performance: ✅ Positive performance impact
Test CoverageAssessment: ✅ Well covered
Final Recommendation✅ APPROVE - This is a high-quality PR with beneficial changes:
All changes align with repository standards and improve the codebase without introducing risks. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR merges changes from the dev branch to main, updating ZenStack client configurations and computed field implementations. The changes focus on simplifying database query configurations and refining computed field parameter handling.
- Removed query logging configuration from sample application
- Updated computed field reference patterns in test schemas
- Added context parameter to computed field type definitions
Reviewed Changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| samples/blog/main.ts | Removed query logging configuration from ZenStackClient setup |
| packages/runtime/test/schemas/typing/typecheck.ts | Simplified whereRef clause by removing table prefix |
| packages/runtime/test/schemas/typing/schema.ts | Added context parameter with currentModel to computed field signature |
| TODO.md | Marked self relation feature as completed |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (3)
TODO.md (1)
80-80: Fix markdown list indentation (MD007) on this linemarkdownlint flagged this item: Expected indent 4; actual 8. Align it with the configured nesting level.
Apply this minimal diff:
- - [x] Self relation + - [x] Self relationNote: Other sibling items under “Misc” also appear indented by 8 spaces; consider running markdownlint --fix or your formatter to normalize the entire section.
packages/runtime/test/schemas/typing/schema.ts (1)
83-85: Optional: strengthen context typing to aid tooling and future readersIf feasible in the generator, consider a named type with a model literal for better inference in downstream usages (kept out of the generated file body, implemented in the generator):
For example, the generator could emit:
type ComputedFieldContext<M extends string> = { currentModel: M };and then for User:
postCount(_context: ComputedFieldContext<'User'>): OperandExpression<number> { ... }This preserves intent and helps editors/tooling.
packages/runtime/test/schemas/typing/typecheck.ts (1)
14-14: Relying on current-model context is good; add a clarifying commentUsing
'id'(unqualified) showcases the new context-based resolution. To aid future readers, add a brief comment noting it refers to User.id via the computed-field context.- .whereRef('Post.authorId', '=', 'id') + .whereRef('Post.authorId', '=', 'id') // 'id' resolves to User.id via computed field context
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (4)
TODO.md(1 hunks)packages/runtime/test/schemas/typing/schema.ts(1 hunks)packages/runtime/test/schemas/typing/typecheck.ts(1 hunks)samples/blog/main.ts(0 hunks)
💤 Files with no reviewable changes (1)
- samples/blog/main.ts
🧰 Additional context used
📓 Path-based instructions (1)
{packages,samples,tests}/**
📄 CodeRabbit Inference Engine (CLAUDE.md)
Packages are located in
packages/,samples/, andtests/
Files:
packages/runtime/test/schemas/typing/typecheck.tspackages/runtime/test/schemas/typing/schema.ts
🪛 markdownlint-cli2 (0.17.2)
TODO.md
80-80: Unordered list indentation
Expected: 4; Actual: 8
(MD007, ul-indent)
⏰ 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: build-test (20.x)
🔇 Additional comments (2)
TODO.md (1)
80-80: Marking “Self relation” as done is fine; ensure coverage across the stackNice to see this checked off. Please confirm we have:
- Unit tests for self-referential to-one and to-many relations (create/connect/disconnect/delete paths, nested mutations).
- Query behaviors covered (include/filter/order/groupBy/aggregate) involving self-relations.
- Typing tests that assert strict types for self-relations in the client.
I can generate a quick script to list existing self-relation tests and highlight gaps if you want.
packages/runtime/test/schemas/typing/schema.ts (1)
83-85: Context-aware computed field signature: LGTMAdding
_contextto thepostCountcomputed field stub is consistent with enabling unqualified refs to resolve against the current model. No functional impact here since this is a stub, but it improves type expressiveness.
Summary by CodeRabbit
New Features
Chores
Documentation
Tests