-
-
Notifications
You must be signed in to change notification settings - Fork 3.5k
feat(server): add morph doc edit tool #12789
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
base: canary
Are you sure you want to change the base?
Conversation
How to use the Graphite Merge QueueAdd either label to this PR to merge it via the merge queue:
You must have a Graphite account in order to use the merge queue. Sign up using this link. An organization admin has enabled the Graphite Merge Queue in this repository. Please do not merge from GitHub as this will restart CI on PRs being processed by the merge queue. |
WalkthroughThis change introduces a new "morph" provider to the copilot plugin, updating configuration schemas, backend provider registration, and frontend configuration. It adds the MorphProvider implementation, integrates a document editing tool, and enhances utility functions and streaming text parsing logic to support the new provider and tool. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant CopilotPlugin
participant MorphProvider
participant MorphAPI
User->>CopilotPlugin: Request text generation or doc edit (with morph provider)
CopilotPlugin->>MorphProvider: Invoke text/streamText or docEdit tool
MorphProvider->>MorphAPI: Send request (with API key and payload)
MorphAPI-->>MorphProvider: Return response (text or stream)
MorphProvider-->>CopilotPlugin: Process and return result/stream
CopilotPlugin-->>User: Deliver response or edit result
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (11)
🚧 Files skipped from review as they are similar to previous changes (11)
⏰ Context from checks skipped due to timeout of 90000ms (55)
✨ Finishing Touches
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. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## canary #12789 +/- ##
==========================================
- Coverage 55.95% 55.94% -0.02%
==========================================
Files 2653 2655 +2
Lines 125675 125948 +273
Branches 19960 19963 +3
==========================================
+ Hits 70322 70461 +139
- Misses 53603 53736 +133
- Partials 1750 1751 +1
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
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: 2
🔭 Outside diff range comments (1)
packages/backend/server/src/plugins/copilot/providers/index.ts (1)
7-19
: 🛠️ Refactor suggestionMissing export for MorphProvider
You’ve imported and registered
MorphProvider
inCopilotProviders
but haven’t re-exported it from this module. Consumers won’t be able to import it alongside the other providers. Add:+export { MorphProvider } from './morph';
below the existing exports.
🧹 Nitpick comments (7)
packages/backend/server/src/plugins/copilot/controller.ts (1)
303-311
: Workspace context passed to streaming generationPassing
workspace: session.config.workspaceId
inprovider.streamText
aligns with the text endpoint. For consistency, consider also propagatingworkspace
in thestreamImages
call withinchatImagesStream
.🧰 Tools
🪛 ESLint
[error] 303-303: Finnish notation should not be used here.
(rxjs/finnish)
packages/backend/server/src/plugins/copilot/providers/types.ts (1)
60-62
: Schema now hard-codes both tools – future additions require code edits
z.enum(['editDoc', 'webSearch'])
forces a code change every time a new tool is introduced.
Consider exposingz.string()
with a runtime guard, or generating the enum from a single source of truth (e.g. the tools registry) to avoid repetitive edits and merge conflicts.packages/backend/server/src/plugins/copilot/providers/utils.ts (2)
12-16
: Import only for types to avoid unnecessary runtime code
createEditDocTool
is used purely for typings insideReturnType
.
Switching to a type-only import prevents the function (and its dependencies) from being bundled at runtime.-import { - createEditDocTool, - createExaCrawlTool, - createExaSearchTool, -} from '../tools'; +import type { createEditDocTool, createExaCrawlTool, createExaSearchTool } from '../tools';
432-438
: Guard against missingresult.result
If the tool accidentally returns
{}
or{ error: ... }
, the current code prints “undefined”, leaking implementation details.- if (chunk.result && typeof chunk.result === 'object') { - result += `\n${chunk.result.result}\n`; - } + if ( + chunk.result && + typeof chunk.result === 'object' && + typeof chunk.result.result === 'string' + ) { + result += `\n${chunk.result.result}\n`; + }packages/backend/server/src/plugins/copilot/providers/anthropic/anthropic.ts (1)
137-161
: Remove unnecessary empty line.The implementation is correct, but there's an unnecessary empty line within the switch case.
case 'webSearch': { tools.web_search_exa = createExaSearchTool(this.AFFiNEConfig); tools.web_crawl_exa = createExaCrawlTool(this.AFFiNEConfig); - break; }
packages/backend/server/src/plugins/copilot/tools/edit-doc.ts (2)
46-49
: Fix typo in parameter description.There's a grammatical error in the instructions parameter description.
.describe( - 'A single sentence instruction describing what you are going to do for the sketched edit. This is used to assist the less intelligent model in applying the edit. Please use the first person to describe what you are going to do. Dont repeat what you have said previously in normal messages. And use it to disambiguate uncertainty in the edit.' + 'A single sentence instruction describing what you are going to do for the sketched edit. This is used to assist the less intelligent model in applying the edit. Please use the first person to describe what you are going to do. Don\'t repeat what you have said previously in normal messages. And use it to disambiguate uncertainty in the edit.' ),
74-76
: Consider logging errors for debugging purposes.The generic catch block swallows error details, which could make debugging difficult in production.
Consider logging the error before returning the generic message:
- } catch { + } catch (error) { + // Log error for debugging while returning user-friendly message + console.error('Failed to apply edit:', error); return 'Failed to apply edit to the doc'; }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
yarn.lock
is excluded by!**/yarn.lock
,!**/*.lock
📒 Files selected for processing (14)
.docker/selfhost/schema.json
(1 hunks)packages/backend/server/package.json
(1 hunks)packages/backend/server/src/plugins/copilot/config.ts
(3 hunks)packages/backend/server/src/plugins/copilot/controller.ts
(2 hunks)packages/backend/server/src/plugins/copilot/prompt/prompts.ts
(1 hunks)packages/backend/server/src/plugins/copilot/providers/anthropic/anthropic.ts
(5 hunks)packages/backend/server/src/plugins/copilot/providers/index.ts
(2 hunks)packages/backend/server/src/plugins/copilot/providers/morph.ts
(1 hunks)packages/backend/server/src/plugins/copilot/providers/openai.ts
(4 hunks)packages/backend/server/src/plugins/copilot/providers/types.ts
(3 hunks)packages/backend/server/src/plugins/copilot/providers/utils.ts
(3 hunks)packages/backend/server/src/plugins/copilot/tools/edit-doc.ts
(1 hunks)packages/backend/server/src/plugins/copilot/tools/index.ts
(1 hunks)packages/frontend/admin/src/config.json
(1 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (4)
packages/backend/server/src/plugins/copilot/controller.ts (1)
packages/backend/server/src/plugins/copilot/resolver.ts (1)
session
(341-359)
packages/backend/server/src/plugins/copilot/providers/index.ts (1)
packages/backend/server/src/plugins/copilot/providers/morph.ts (1)
MorphProvider
(27-163)
packages/backend/server/src/plugins/copilot/config.ts (1)
packages/backend/server/src/plugins/copilot/providers/morph.ts (1)
MorphConfig
(23-25)
packages/backend/server/src/plugins/copilot/providers/openai.ts (1)
packages/backend/server/src/plugins/copilot/tools/edit-doc.ts (2)
buildContentGetter
(8-23)createEditDocTool
(25-79)
⏰ Context from checks skipped due to timeout of 90000ms (53)
- GitHub Check: test-build-mobile-app / build-ios-web
- GitHub Check: test-build-mobile-app / build-android-web
- GitHub Check: y-octo binding test on aarch64-pc-windows-msvc
- GitHub Check: E2E BlockSuite Test (10)
- GitHub Check: y-octo binding test on aarch64-apple-darwin
- GitHub Check: y-octo binding test on x86_64-pc-windows-msvc
- GitHub Check: fuzzing
- GitHub Check: E2E BlockSuite Test (2)
- GitHub Check: y-octo binding test on x86_64-apple-darwin
- GitHub Check: loom thread test
- GitHub Check: E2E BlockSuite Test (5)
- GitHub Check: E2E BlockSuite Test (7)
- GitHub Check: E2E BlockSuite Test (6)
- GitHub Check: E2E BlockSuite Test (9)
- GitHub Check: Build AFFiNE native (x86_64-apple-darwin)
- GitHub Check: E2E BlockSuite Test (8)
- GitHub Check: Run native tests
- GitHub Check: E2E BlockSuite Test (1)
- GitHub Check: E2E BlockSuite Cross Browser Test (1, webkit)
- GitHub Check: E2E BlockSuite Test (4)
- GitHub Check: E2E BlockSuite Cross Browser Test (2, webkit)
- GitHub Check: E2E BlockSuite Test (3)
- GitHub Check: Build AFFiNE native (aarch64-apple-darwin)
- GitHub Check: E2E BlockSuite Cross Browser Test (2, firefox)
- GitHub Check: E2E BlockSuite Cross Browser Test (1, firefox)
- GitHub Check: E2E Mobile Test (5)
- GitHub Check: E2E Mobile Test (2)
- GitHub Check: E2E BlockSuite Cross Browser Test (2, chromium)
- GitHub Check: E2E Mobile Test (1)
- GitHub Check: E2E Mobile Test (3)
- GitHub Check: E2E BlockSuite Cross Browser Test (1, chromium)
- GitHub Check: E2E Mobile Test (4)
- GitHub Check: E2E Test (6)
- GitHub Check: E2E Test (5)
- GitHub Check: E2E Test (8)
- GitHub Check: E2E Test (4)
- GitHub Check: Build Server native
- GitHub Check: E2E Test (9)
- GitHub Check: E2E Test (10)
- GitHub Check: E2E Test (3)
- GitHub Check: E2E Test (1)
- GitHub Check: E2E Test (7)
- GitHub Check: E2E Test (2)
- GitHub Check: Build @affine/electron renderer
- GitHub Check: Build AFFiNE native (x86_64-pc-windows-msvc)
- GitHub Check: Typecheck
- GitHub Check: Analyze (typescript, affine)
- GitHub Check: Build AFFiNE native (aarch64-pc-windows-msvc)
- GitHub Check: Analyze (javascript, affine)
- GitHub Check: Analyze (javascript, blocksuite)
- GitHub Check: Lint
- GitHub Check: Analyze (typescript, blocksuite)
- GitHub Check: Socket Security: Pull Request Alerts
🔇 Additional comments (13)
packages/backend/server/package.json (1)
35-35
: Addopenai-compatible
dependency for MorphProvider integrationIntroducing
@ai-sdk/openai-compatible
is required for your new MorphProvider. Ensure you update the lockfile (yarn.lock
/package-lock.json
) and runyarn install
ornpm install
in CI to pull in the new package.packages/backend/server/src/plugins/copilot/controller.ts (1)
233-240
: Workspace context passed to text generationAdding
workspace: session.config.workspaceId
to theprovider.text
call ensures the provider has the correct workspace scope for doc edits. Please verify thatsession.config.workspaceId
is always set when a session is created.packages/backend/server/src/plugins/copilot/tools/index.ts (1)
1-1
: Re-exportedit-doc
toolExporting
./edit-doc
correctly surfaces your new editDoc tool in the public API. Confirm that any documentation and GraphQL schemas are updated to reference this new export..docker/selfhost/schema.json (1)
730-734
: Add Morph provider config schemaThe addition of
"providers.morph"
follows the existing pattern. Please run your JSON schema validator to confirm acceptance and verify that the frontend config surfaces this new property.packages/backend/server/src/plugins/copilot/providers/types.ts (1)
125-126
: Double-check workspace propagation
workspace
is added toCopilotProviderOptionsSchema
, but not all providers implement or forward it yet.
Please verify each provider’s option handling to preventunknown option
warnings or silent drops.packages/frontend/admin/src/config.json (1)
252-255
: Morph provider entry looks goodEntry is syntactically correct and consistent with existing provider blocks.
packages/backend/server/src/plugins/copilot/providers/openai.ts (2)
69-71
: LGTM! Proper dependency injection pattern.The injected dependencies follow the NestJS pattern correctly with appropriate use of the non-null assertion operator for framework-injected properties.
267-274
: Tool integration looks good.The
editDoc
tool case follows the same pattern as the existingwebSearch
case, properly utilizing the injected dependencies and binding the options context.packages/backend/server/src/plugins/copilot/providers/anthropic/anthropic.ts (1)
32-34
: Consistent dependency injection across providers.The injection pattern matches the OpenAI provider implementation, maintaining consistency across the codebase.
packages/backend/server/src/plugins/copilot/providers/morph.ts (3)
27-55
: Provider setup looks good.The Morph provider is correctly configured with appropriate model capabilities and OpenAI-compatible client setup.
75-106
: Text generation method follows established patterns.The implementation correctly handles parameter validation, metrics tracking, and error handling consistent with other providers.
108-163
: Stream handling implementation is comprehensive.The method properly handles different chunk types, citation parsing, and stream cancellation, matching the pattern used in the OpenAI provider.
packages/backend/server/src/plugins/copilot/tools/edit-doc.ts (1)
8-23
: Excellent security implementation with proper access control.The function correctly validates all required parameters and enforces document read permissions before retrieving content. The graceful handling of missing parameters and access denial is well implemented.
f08dd24
to
573d931
Compare
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: 2
♻️ Duplicate comments (1)
packages/backend/server/src/plugins/copilot/prompt/prompts.ts (1)
1794-1800
: Name mismatch will still break tool dispatch – usesnake_case
config.tools
now lists"docEdit"
,"docRead"
… while the provider / parser side usesdoc_edit
,doc_read
, etc.
Unless a canonicalisation layer has been added elsewhere, the assistant will still never emit – and the client will never understand – the correct tool name, so the feature remains dead-on-arrival.- tools: [ - 'docRead', - 'docEdit', - 'docKeywordSearch', - 'docSemanticSearch', - 'webSearch', - ], + tools: [ + 'doc_read', + 'doc_edit', + 'doc_keyword_search', + 'doc_semantic_search', + 'webSearch', + ],
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
packages/backend/server/src/plugins/copilot/prompt/prompts.ts
(1 hunks)packages/backend/server/src/plugins/copilot/providers/provider.ts
(2 hunks)packages/backend/server/src/plugins/copilot/providers/types.ts
(2 hunks)packages/backend/server/src/plugins/copilot/providers/utils.ts
(3 hunks)packages/backend/server/src/plugins/copilot/tools/doc-edit.ts
(1 hunks)packages/backend/server/src/plugins/copilot/tools/index.ts
(1 hunks)packages/backend/server/src/plugins/copilot/tools/semantic-search.ts
(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- packages/backend/server/src/plugins/copilot/tools/semantic-search.ts
🚧 Files skipped from review as they are similar to previous changes (4)
- packages/backend/server/src/plugins/copilot/tools/index.ts
- packages/backend/server/src/plugins/copilot/providers/provider.ts
- packages/backend/server/src/plugins/copilot/providers/utils.ts
- packages/backend/server/src/plugins/copilot/providers/types.ts
⏰ Context from checks skipped due to timeout of 90000ms (80)
- GitHub Check: Frontend Copilot E2E Test (3, 8)
- GitHub Check: Frontend Copilot E2E Test (4, 8)
- GitHub Check: Cloud E2E Test 6/6
- GitHub Check: Cloud E2E Test 3/6
- GitHub Check: Cloud E2E Test 1/6
- GitHub Check: Frontend Copilot E2E Test (6, 8)
- GitHub Check: Server Test (7, 8)
- GitHub Check: Cloud Desktop E2E Test
- GitHub Check: Cloud E2E Test 2/6
- GitHub Check: Cloud E2E Test 5/6
- GitHub Check: Server Test (4, 8)
- GitHub Check: Server Copilot Api Test
- GitHub Check: Frontend Copilot E2E Test (5, 8)
- GitHub Check: Server Test (3, 8)
- GitHub Check: Cloud E2E Test 4/6
- GitHub Check: Frontend Copilot E2E Test (8, 8)
- GitHub Check: Frontend Copilot E2E Test (2, 8)
- GitHub Check: Server Test (5, 8)
- GitHub Check: Server Test (0, 8)
- GitHub Check: Server Test (6, 8)
- GitHub Check: Frontend Copilot E2E Test (1, 8)
- GitHub Check: Frontend Copilot E2E Test (7, 8)
- GitHub Check: Server Test (1, 8)
- GitHub Check: Server Test (2, 8)
- GitHub Check: Server Test with Elasticsearch
- GitHub Check: Server E2E Test
- GitHub Check: Check Git Status
- GitHub Check: Unit Test (5)
- GitHub Check: Unit Test (3)
- GitHub Check: Unit Test (1)
- GitHub Check: Unit Test (4)
- GitHub Check: Unit Test (2)
- GitHub Check: test-build-mobile-app / build-ios-web
- GitHub Check: test-build-mobile-app / build-android-web
- GitHub Check: Build AFFiNE native (aarch64-pc-windows-msvc)
- GitHub Check: E2E BlockSuite Test (4)
- GitHub Check: y-octo binding test on aarch64-pc-windows-msvc
- GitHub Check: E2E BlockSuite Cross Browser Test (1, chromium)
- GitHub Check: Run native tests
- GitHub Check: y-octo binding test on x86_64-pc-windows-msvc
- GitHub Check: E2E BlockSuite Test (10)
- GitHub Check: loom thread test
- GitHub Check: E2E BlockSuite Test (1)
- GitHub Check: Build AFFiNE native (x86_64-pc-windows-msvc)
- GitHub Check: E2E BlockSuite Test (2)
- GitHub Check: E2E BlockSuite Test (9)
- GitHub Check: E2E Test (8)
- GitHub Check: miri code check
- GitHub Check: E2E Test (9)
- GitHub Check: E2E BlockSuite Test (8)
- GitHub Check: fuzzing
- GitHub Check: E2E Test (10)
- GitHub Check: E2E BlockSuite Cross Browser Test (1, webkit)
- GitHub Check: y-octo binding test on x86_64-apple-darwin
- GitHub Check: E2E BlockSuite Cross Browser Test (2, chromium)
- GitHub Check: E2E BlockSuite Cross Browser Test (2, firefox)
- GitHub Check: E2E BlockSuite Cross Browser Test (2, webkit)
- GitHub Check: E2E BlockSuite Test (7)
- GitHub Check: E2E BlockSuite Test (6)
- GitHub Check: E2E BlockSuite Test (5)
- GitHub Check: E2E BlockSuite Cross Browser Test (1, firefox)
- GitHub Check: E2E BlockSuite Test (3)
- GitHub Check: E2E Test (2)
- GitHub Check: E2E Test (4)
- GitHub Check: E2E Test (1)
- GitHub Check: E2E Test (3)
- GitHub Check: E2E Mobile Test (2)
- GitHub Check: E2E Test (6)
- GitHub Check: E2E Mobile Test (5)
- GitHub Check: E2E Test (5)
- GitHub Check: E2E Mobile Test (4)
- GitHub Check: E2E Test (7)
- GitHub Check: Analyze (javascript, blocksuite)
- GitHub Check: E2E Mobile Test (1)
- GitHub Check: E2E Mobile Test (3)
- GitHub Check: Analyze (javascript, affine)
- GitHub Check: Analyze (typescript, affine)
- GitHub Check: Build @affine/electron renderer
- GitHub Check: Analyze (typescript, blocksuite)
- GitHub Check: Typecheck
🔇 Additional comments (1)
packages/backend/server/src/plugins/copilot/tools/doc-edit.ts (1)
67-74
: Return type may not match the tool runtime contract
execute
currently returns{ result }
, i.e. an object containing the provider’s reply.
If the surrounding runtime expects a plain string (manyai.tool()
wrappers do), this will surface as[object Object]
or throw.Confirm the expected shape and, if needed, return
result
directly:- const result = await provider.text(/* … */); - return { result }; + return await provider.text(/* … */);
2a22d4a
to
9c8b847
Compare
9c8b847
to
3939e7f
Compare
Summary by CodeRabbit