Introduce network request handler#4194
Conversation
There was a problem hiding this comment.
Code Review
This pull request migrates the font catalog loading and resource resolution logic from the Svelte frontend to the Rust backend, introducing a new Network message handler that uses reqwest to fetch resources asynchronously. Feedback has been provided to address three key issues: first, the fetch method should check for HTTP success status codes using response.error_for_status() to prevent caching corrupted error pages; second, the font variant parsing logic needs to strip the "italic" suffix before parsing the weight to avoid silently skipping non-400 italic variants; and third, the dynamically loaded font catalog should be dispatched back to the system via FontsMessage::CatalogLoaded so it is saved in the application state rather than being discarded and re-fetched on subsequent requests.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
There was a problem hiding this comment.
7 issues found across 23 files
Confidence score: 2/5
- High merge risk: there are multiple concrete, high-confidence runtime issues (sev 7-8) affecting request execution and resource loading behavior, so this is not yet a safe merge despite being fixable.
- Most severe functional issue is in
editor/src/messages/network/network_message.rs: cloningNetworkMessage::Requestdrops the request closure, which can turn valid requests into empty-request error handling and silently break network operations. - Resource resolution has likely regressions in
editor/src/messages/portfolio/document_migration.rsandeditor/src/messages/portfolio/document/resource/resource_message_handler.rs(incorrect embedded source registration, and an unconditional break that prevents fallback sources after fetch failure), pluseditor/src/messages/network/utility_types.rscan hang flows due to missing client timeouts. - Pay close attention to
editor/src/messages/network/network_message.rs,editor/src/messages/portfolio/document_migration.rs,editor/src/messages/portfolio/document/resource/resource_message_handler.rs, andeditor/src/messages/network/utility_types.rs- these are the most likely to cause user-visible request/resource failures or stalls.
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="editor/src/messages/portfolio/portfolio_message_handler.rs">
<violation number="1" location="editor/src/messages/portfolio/portfolio_message_handler.rs:426">
P2: Unconditional `ResolveAll` can trigger repeated font-catalog network loads when catalog is empty. This regresses startup/resource-resolve performance and can cause avoidable duplicate API calls.</violation>
</file>
<file name="editor/src/messages/portfolio/fonts/fonts_message_handler.rs">
<violation number="1" location="editor/src/messages/portfolio/fonts/fonts_message_handler.rs:26">
P2: LoadCatalog requests are not deduplicated, so startup/tool events can trigger parallel catalog fetches. This adds avoidable network load and nondeterministic last-response-wins catalog updates.</violation>
</file>
<file name="editor/src/messages/portfolio/document/resource/resource_message.rs">
<violation number="1" location="editor/src/messages/portfolio/document/resource/resource_message.rs:2">
P1: Custom agent: **PR title enforcement**
PR title is not in imperative mood and lacks a leading action verb required by the PR title convention.</violation>
</file>
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
| @@ -1,5 +1,5 @@ | |||
| use crate::messages::prelude::*; | |||
There was a problem hiding this comment.
P1: Custom agent: PR title enforcement
PR title is not in imperative mood and lacks a leading action verb required by the PR title convention.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At editor/src/messages/portfolio/document/resource/resource_message.rs, line 2:
<comment>PR title is not in imperative mood and lacks a leading action verb required by the PR title convention.</comment>
<file context>
@@ -1,5 +1,5 @@
use crate::messages::prelude::*;
-use graph_craft::application_io::resource::ResourceId;
+use graph_craft::application_io::resource::{DataSource, ResourceHash, ResourceId};
use graphene_std::text::Font;
use std::sync::Arc;
</file context>
| let FontsMessageContext { resource_storage } = context; | ||
|
|
||
| match message { | ||
| FontsMessage::LoadCatalog => { |
There was a problem hiding this comment.
P2: LoadCatalog requests are not deduplicated, so startup/tool events can trigger parallel catalog fetches. This adds avoidable network load and nondeterministic last-response-wins catalog updates.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At editor/src/messages/portfolio/fonts/fonts_message_handler.rs, line 26:
<comment>LoadCatalog requests are not deduplicated, so startup/tool events can trigger parallel catalog fetches. This adds avoidable network load and nondeterministic last-response-wins catalog updates.</comment>
<file context>
@@ -22,8 +23,17 @@ impl MessageHandler<FontsMessage, FontsMessageContext<'_>> for FontsMessageHandl
let FontsMessageContext { resource_storage } = context;
match message {
+ FontsMessage::LoadCatalog => {
+ responses.add(NetworkMessage::request(|client| async move {
+ let Some(catalog) = FontCatalog::load_from_api(&client).await else {
</file context>
| responses.add(PortfolioMessage::DocumentPassMessage { | ||
| document_id, | ||
| message: DocumentMessage::Resource(ResourceMessage::Resolve), | ||
| message: DocumentMessage::Resource(ResourceMessage::ResolveAll), |
There was a problem hiding this comment.
P2: Unconditional ResolveAll can trigger repeated font-catalog network loads when catalog is empty. This regresses startup/resource-resolve performance and can cause avoidable duplicate API calls.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At editor/src/messages/portfolio/portfolio_message_handler.rs, line 426:
<comment>Unconditional `ResolveAll` can trigger repeated font-catalog network loads when catalog is empty. This regresses startup/resource-resolve performance and can cause avoidable duplicate API calls.</comment>
<file context>
@@ -422,14 +421,9 @@ impl MessageHandler<PortfolioMessage, PortfolioMessageContext<'_>> for Portfolio
responses.add(PortfolioMessage::DocumentPassMessage {
document_id,
- message: DocumentMessage::Resource(ResourceMessage::Resolve),
+ message: DocumentMessage::Resource(ResourceMessage::ResolveAll),
});
}
</file context>
There was a problem hiding this comment.
1 issue found across 4 files (changes from recent commits).
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="editor/src/messages/portfolio/portfolio_message_handler.rs">
<violation number="1" location="editor/src/messages/portfolio/portfolio_message_handler.rs:426">
P2: Unconditional `ResolveAll` can trigger repeated font-catalog network loads when catalog is empty. This regresses startup/resource-resolve performance and can cause avoidable duplicate API calls.</violation>
</file>
<file name="editor/src/messages/portfolio/fonts/fonts_message_handler.rs">
<violation number="1" location="editor/src/messages/portfolio/fonts/fonts_message_handler.rs:26">
P2: LoadCatalog requests are not deduplicated, so startup/tool events can trigger parallel catalog fetches. This adds avoidable network load and nondeterministic last-response-wins catalog updates.</violation>
</file>
<file name="editor/src/messages/portfolio/document/resource/resource_message.rs">
<violation number="1" location="editor/src/messages/portfolio/document/resource/resource_message.rs:2">
P1: Custom agent: **PR title enforcement**
PR title is not in imperative mood and lacks a leading action verb required by the PR title convention.</violation>
</file>
Tip: Review your code locally with the cubic CLI to iterate faster.
Re-trigger cubic
* Impl NetworkMessageHandler * Repalce Frontend fetch like messages with NetworkMessage * Reimpl resource loading with NetworkMessage * Fix wasm * dedublicate resource requests * Embedd font resources created by migration * Fixup * Fix tests * Fix font catalog not loading * Cleanup * Fix layouts not updating when font catalog is loaded * Review * Review * Cleanup
No description provided.