Am2 linking prop wip #2155
Am2 linking prop wip #2155
Conversation
✅ Deploy Preview for hyprnote-storybook ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
✅ Deploy Preview for hyprnote ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
📝 WalkthroughWalkthroughThis pull request updates the Swift framework build configuration across multiple Cargo.toml and build.rs files to conditionally link Swift frameworks on macOS aarch64, upgrades the swift-rs workspace dependency, and refactors the AssemblyAI audio upload to read files directly with inferred content-types instead of decoding bytes. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes
Pre-merge checks and finishing touches❌ Failed checks (2 warnings, 1 inconclusive)
✨ 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (1)
owhisper/owhisper-client/src/adapter/assemblyai/batch.rs (1)
118-126: Consider using a more robust Content-Type detection mechanism.While extension-based Content-Type inference works for common cases, it may fail if:
- Files have incorrect extensions
- Less common audio formats are used (e.g., aac, opus, wma)
The current implementation is functional, but consider using a library like
inferormime_guessfor more robust detection based on file content rather than just extension.Example using
mime_guess:+use mime_guess; + - let content_type = match file_path.extension().and_then(|e| e.to_str()) { - Some("wav") => "audio/wav", - Some("mp3") => "audio/mpeg", - Some("ogg") => "audio/ogg", - Some("flac") => "audio/flac", - Some("m4a") => "audio/mp4", - Some("webm") => "audio/webm", - _ => "application/octet-stream", - }; + let content_type = mime_guess::from_path(&file_path) + .first_or_octet_stream() + .to_string();
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (9)
Cargo.toml(1 hunks)apps/desktop/src-tauri/Cargo.toml(1 hunks)apps/desktop/src-tauri/build.rs(1 hunks)crates/vad-ext/Cargo.toml(1 hunks)crates/vad-ext/build.rs(1 hunks)crates/vvad/build.rs(1 hunks)owhisper/owhisper-client/src/adapter/assemblyai/batch.rs(1 hunks)plugins/local-stt/Cargo.toml(2 hunks)plugins/local-stt/build.rs(1 hunks)
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-11-27T11:40:22.782Z
Learnt from: CR
Repo: fastrepl/hyprnote PR: 0
File: plugins/AGENTS.md:0-0
Timestamp: 2025-11-27T11:40:22.782Z
Learning: Applies to plugins/*/src/lib.rs : After updating commands in `plugins/<NAME>/src/lib.rs`, run `codegen`, update `plugins/<NAME>/permissions/default.toml`, and `apps/desktop/src-tauri/capabilities/default.json`
Applied to files:
plugins/local-stt/Cargo.tomlplugins/local-stt/build.rs
⏰ 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). (7)
- GitHub Check: Redirect rules - hyprnote
- GitHub Check: Header rules - hyprnote
- GitHub Check: Pages changed - hyprnote
- GitHub Check: desktop_ci (linux, depot-ubuntu-24.04-8)
- GitHub Check: desktop_ci (macos, depot-macos-15)
- GitHub Check: desktop_ci (linux, depot-ubuntu-22.04-8)
- GitHub Check: fmt
🔇 Additional comments (7)
plugins/local-stt/Cargo.toml (2)
25-27: LGTM!The target-specific build-dependency for swift-rs on macOS aarch64 is correctly configured and aligns with the Swift framework linking added in build.rs.
50-50: LGTM!The addition of hypr-vad-ext as a runtime dependency is appropriate and supports the VAD integration for the local STT plugin.
crates/vvad/build.rs (1)
1-4: LGTM!The switch from
propagate_framework_rpathtolink_swift_frameworkis consistent with the broader PR changes to standardize Swift framework linking on macOS aarch64.crates/vad-ext/build.rs (1)
1-4: LGTM!The switch from
propagate_framework_rpathtolink_swift_frameworkaligns with the PR's objective to standardize Swift framework linking on macOS aarch64.Cargo.toml (1)
260-260: The swift-rs revision 0b36e41 appears valid based on codebase analysis. Cargo.lock successfully resolves this revision to the full commit hash0b36e4146974b26c68677e12cf5b1700eb94edcf, confirming the revision exists in the repository. Thelink_swift_frameworkfunctionality is actively used across four build scripts (plugins/local-stt, crates/vvad, crates/vad-ext, and apps/desktop/src-tauri), all callingswift_rs::link_swift_framework()without apparent issues. No breaking changes are evident from the workspace dependency usage.owhisper/owhisper-client/src/adapter/assemblyai/batch.rs (1)
114-135: This approach aligns with AssemblyAI's recommended best practice.AssemblyAI explicitly recommends submitting audio in its native format without client-side transcoding. The API accepts all the formats in the Content-Type mapping (wav, mp3, ogg, flac, m4a, webm) and handles transcoding server-side (converting to 16 kHz uncompressed audio). Uploading raw files directly is the correct approach per AssemblyAI documentation.
Likely an incorrect or invalid review comment.
apps/desktop/src-tauri/Cargo.toml (1)
22-23: Verify the rationale for aarch64-only scoping and confirm this dependency is actively used.The target-specific build-dependency is syntactically correct and properly references the workspace-defined swift-rs. However, without build-time implementation details, the scope of this conditional (aarch64 only, not x86_64 macOS) cannot be verified. Clarify whether both macOS architectures require this dependency or if aarch64-specific linking is intentional.
| #[cfg(all(target_os = "macos", target_arch = "aarch64"))] | ||
| swift_rs::link_swift_framework("tauri-plugin-local-stt"); | ||
|
|
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check the links field in local-stt plugin Cargo.toml
echo "=== Checking links field in plugins/local-stt/Cargo.toml ==="
cat plugins/local-stt/Cargo.toml | grep -A 1 'links ='
# Check if there are any Swift source files in the plugin
echo -e "\n=== Checking for Swift source files ==="
fd -e swift . plugins/local-stt/ 2>/dev/null || echo "No Swift files found"
# Check the build.rs file content
echo -e "\n=== Checking build.rs file ==="
cat -n apps/desktop/src-tauri/build.rs | head -20
# Look for any other references to this framework name
echo -e "\n=== Checking for other references to tauri-plugin-local-stt ==="
rg "tauri-plugin-local-stt" --type rust --type tomlRepository: fastrepl/hyprnote
Length of output: 1584
🏁 Script executed:
#!/bin/bash
# Check if there's a pre-built framework or binary somewhere
echo "=== Looking for framework files or Swift build artifacts ==="
fd "\.framework" plugins/local-stt/ 2>/dev/null || echo "No .framework directories found"
fd "\.a$|\.dylib$|\.so$" plugins/local-stt/ 2>/dev/null || echo "No library files found"
# Check the plugin's build.rs if it exists
echo -e "\n=== Checking if plugin has build.rs ==="
ls -la plugins/local-stt/build.rs 2>/dev/null || echo "No build.rs in plugin"
# Check the plugin structure
echo -e "\n=== Plugin directory structure ==="
find plugins/local-stt -type f -name "*.rs" -o -name "*.swift" -o -name "*.toml" | head -20
# Check src directory
echo -e "\n=== Plugin src directory ==="
ls -la plugins/local-stt/src/ 2>/dev/null || echo "No src directory"
# Look at the plugin's Cargo.toml more thoroughly
echo -e "\n=== Full Cargo.toml of plugin ==="
head -30 plugins/local-stt/Cargo.tomlRepository: fastrepl/hyprnote
Length of output: 2356
🏁 Script executed:
#!/bin/bash
# Read the plugin's build.rs
echo "=== Plugin's build.rs ==="
cat -n plugins/local-stt/build.rs
# Read more of the desktop app's build.rs to understand the full context
echo -e "\n=== Desktop app's full build.rs ==="
cat -n apps/desktop/src-tauri/build.rs
# Check workspace Cargo.toml to understand dependency order
echo -e "\n=== Checking workspace members and build order ==="
cat Cargo.toml | grep -A 20 "\[workspace\]"Repository: fastrepl/hyprnote
Length of output: 2378
Fix framework name mismatch: plugin's build.rs links "vad-ext" but should link "tauri-plugin-local-stt".
The plugin's Cargo.toml declares links = "tauri-plugin-local-stt" (line 7), and the desktop app's build.rs attempts to link a framework with that same name (line 6 of apps/desktop/src-tauri/build.rs). However, the plugin's own build.rs (line 16 of plugins/local-stt/build.rs) links swift_rs::link_swift_framework("vad-ext") instead. This mismatch will cause a build failure because:
- The
linksfield in Cargo.toml must match what the build.rs actually produces - The desktop app expects to link "tauri-plugin-local-stt" but the plugin creates "vad-ext"
Update plugins/local-stt/build.rs line 16 to link the correct framework name that matches the Cargo.toml links field:
swift_rs::link_swift_framework("tauri-plugin-local-stt");🤖 Prompt for AI Agents
In plugins/local-stt/build.rs around line 16, the build script links the Swift
framework as "vad-ext" which mismatches the Cargo.toml links field
("tauri-plugin-local-stt"); change the call to swift_rs::link_swift_framework to
link "tauri-plugin-local-stt" so the produced framework name matches Cargo.toml
and the desktop app's build.rs.
| name = "vad-ext" | ||
| version = "0.1.0" | ||
| edition = "2021" | ||
| links = "vad-ext" |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
Cargo links key metadata native library linking best practices
💡 Result:
Concise best-practices for using Cargo's package.links metadata and linking native libraries
-
Use links only for low-level "sys" crates; keep high-level safe wrappers free of links. This follows the common sys / non-sys pattern. [1][4]
-
Always include a build script when you set package.links. The build script must emit rustc-link-lib / rustc-link-search (cargo:rustc-link-lib / cargo:rustc-link-search) and may emit metadata via cargo:metadata for dependents. [1][3]
-
Ensure each links name is unique across your dependency graph (one package per links name). Cargo enforces this to avoid duplicate native symbols; avoid semver-incompatible releases that change behavior for widely-used links names. [2]
-
Export metadata carefully and predictably. Build-script metadata is passed to immediate dependents as DEP__=value environment variables — not to transitive dependents — so design your sys crate API and metadata keys accordingly. [3]
-
Allow users / packagers to override build-script behavior via .cargo/config (rustc-link-lib, rustc-link-search, root, metadata keys). Document how to configure overrides for distro/system-provided libraries. [1][4]
-
Prefer static vs dynamic explicitly: emit rustc-link-lib with the appropriate kind (static/dylib/framework) in the build script; document expected linkage and required linker flags. [1][7]
-
Minimize global side effects: avoid emitting linker args or env cfgs that change global build behavior for unrelated crates; confine changes to what consumers need. [1][7]
-
Test dependency resolution scenarios: CI should build with realistic graphs (multiple versions of wrappers) to catch "multiple packages link to native library" errors early; if necessary, recommend consumers pin or unify versions. [2][8]
Primary references
- Cargo build scripts / links key (how to emit rustc-link-*, metadata, overrides). [1][4]
- Dependency resolver rules for links (one crate per links name, resolver errors). [2]
- Build-script metadata / DEP_* variable behavior and usage. [3]
If you want, I can produce a minimal example sys crate (Cargo.toml + build.rs) showing rustc-link-lib/search + metadata and a small non-sys wrapper that consumes DEP_* values.
🏁 Script executed:
# First, let's examine the build.rs file to understand what's actually being linked
cat -n crates/vad-ext/build.rsRepository: fastrepl/hyprnote
Length of output: 208
🏁 Script executed:
# Check the Cargo.toml structure and metadata
cat -n crates/vad-ext/Cargo.tomlRepository: fastrepl/hyprnote
Length of output: 999
🏁 Script executed:
# Search for any usage of DEP_VAD_EXT in the codebase
rg "DEP_VAD_EXT" --type rust --type tomlRepository: fastrepl/hyprnote
Length of output: 43
🏁 Script executed:
# Check if other crates depend on vad-ext and what they expect
rg "vad-ext" --type toml -A 2 -B 2Repository: fastrepl/hyprnote
Length of output: 2839
Remove the links field from Cargo.toml.
The links metadata is intended only for low-level "sys" crates that provide direct native bindings. vad-ext is a high-level wrapper with regular Rust dependencies (serde, tokio, thiserror, etc.), not a sys crate. Sys crates use links to declare and communicate linking metadata to dependents via build scripts; since vad-ext doesn't follow this pattern and no downstream crates consume DEP_VAD_EXT_* variables, this field should not be present.
🤖 Prompt for AI Agents
crates/vad-ext/Cargo.toml around line 5: the file currently contains a `links =
"vad-ext"` metadata entry which is only appropriate for low-level sys crates
exposing native bindings; remove the `links` field from Cargo.toml to reflect
that vad-ext is a high-level wrapper with normal Rust dependencies and does not
provide or export build-script linking variables.
| #[cfg(all(target_os = "macos", target_arch = "aarch64"))] | ||
| swift_rs::link_swift_framework("vad-ext"); | ||
|
|
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
find . -name "build.rs" | head -20Repository: fastrepl/hyprnote
Length of output: 599
🏁 Script executed:
cat -n crates/vad-ext/build.rs 2>/dev/null || echo "File not found, searching..."Repository: fastrepl/hyprnote
Length of output: 208
🏁 Script executed:
find . -path "*/vad-ext/*" -name "build.rs" -exec cat -n {} \;Repository: fastrepl/hyprnote
Length of output: 208
🏁 Script executed:
find . -path "*/vad-ext/*" -type f \( -name "Cargo.toml" -o -name "build.rs" \) | head -10Repository: fastrepl/hyprnote
Length of output: 114
🏁 Script executed:
cat -n crates/vad-ext/Cargo.tomlRepository: fastrepl/hyprnote
Length of output: 999
🏁 Script executed:
cat -n plugins/local-stt/Cargo.tomlRepository: fastrepl/hyprnote
Length of output: 3557
🏁 Script executed:
cat -n crates/vvad/Cargo.toml 2>/dev/null || echo "File not found"Repository: fastrepl/hyprnote
Length of output: 600
🏁 Script executed:
find . -name "*.swift" -o -name "*.xcframework" | grep -i "vad" | head -20Repository: fastrepl/hyprnote
Length of output: 43
🏁 Script executed:
find . -type d -name "*.framework" -o -name "*.xcframework" | head -20Repository: fastrepl/hyprnote
Length of output: 43
🏁 Script executed:
find crates/vad-ext -type f | head -30Repository: fastrepl/hyprnote
Length of output: 257
🏁 Script executed:
cat -n plugins/local-stt/build.rsRepository: fastrepl/hyprnote
Length of output: 637
🏁 Script executed:
find plugins/local-stt -type f -name "*.rs" | head -10Repository: fastrepl/hyprnote
Length of output: 406
Change framework name from "vad-ext" to "vvad".
The build script attempts to link a Swift framework named "vad-ext", but no such framework exists in the codebase. The vad-ext crate is a Rust wrapper that itself links the actual Swift framework "vvad" (see crates/vad-ext/build.rs line 3). Update line 16 to link the correct framework:
#[cfg(all(target_os = "macos", target_arch = "aarch64"))]
swift_rs::link_swift_framework("vvad");
🤖 Prompt for AI Agents
In plugins/local-stt/build.rs around lines 15 to 17, the script links a
non-existent Swift framework "vad-ext"; change the linked framework name to
"vvad" by updating the swift_rs::link_swift_framework call to use "vvad" so it
matches the actual framework the vad-ext crate wraps.
No description provided.