feat(plugin): SoundCloud, Vimeo, and Gallery WASM plugins (task 25)#24
Merged
feat(plugin): SoundCloud, Vimeo, and Gallery WASM plugins (task 25)#24
Conversation
Task 25 — three new WASM plugins under plugins/, each delegating network I/O to the host http_request host function and sharing the same layout as vortex-mod-youtube (pure logic in sibling modules, WASM-only plugin_api.rs gated behind cfg(target_family = "wasm")). * SoundCloud: /resolve API v2 client with tagged enum ResolveResponse (Track / Playlist / User / Unknown), URL router covering sets, likes, tracks, albums and m./on. hosts, artwork upgrade from -large to -t500x500 (handles .ext, no-ext and query-string variants), client_id forwarded via get_config, 41 native unit tests. * Vimeo: oEmbed JSON client for metadata plus player config JSON client (/video/<id>/config) for progressive MP4 + HLS variants. Balanced-brace HTML fallback handles both single- and double-quoted strings. pick_variant_for_quality with 2K->1440 / 4K->2160 mapping, filter_audio_only preserving HLS, recognises public / private-hash / showcase shapes, 46 native unit tests. * Gallery: Imgur v3 album API, Reddit submission JSON with native gallery + single-image preview fallback (& unescaping, deterministic URL sort), Flickr photosets.getPhotos with url-encoded api_key. Generic <img> scraper exposed as separate extract_generic export so the plugin does not claim every HTTP page. Scheme guard blocks data:/javascript:/mailto:/blob: from relative resolution. Post-processing: dedupe, min-resolution filter (preserves unknown dimensions), auto-name with album-id sanitisation, 46 native tests. * Shared hardening: per-crate PluginError via thiserror with SerdeJson #[from], no .unwrap() in production paths, no #[allow(dead_code)], every unsafe host-fn call site has a documented SAFETY invariant block. Clippy -D warnings clean on both wasm32-wasip1 and x86_64-unknown-linux-gnu. Release WASM sizes (opt-level = "z", LTO, strip): SoundCloud 256 KB, Vimeo 1.17 MB, Gallery 1.19 MB.
github-actions
bot
added
the
documentation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds three new WASM plugins—Gallery, SoundCloud, and Vimeo—introducing crate manifests/Cargo configs, plugin TOMLs, provider URL classification, HTTP host-envelope handling, provider parsers/request builders, post-processing (dedupe/filter/auto-name/variant selection), Extism host wiring, docs, and unit tests. Changes
Sequence DiagramsequenceDiagram
participant Client
participant Plugin as WASM Plugin
participant Host as Vortex Host
participant API as External Provider API
Client->>Plugin: extract_links(url)
Plugin->>Plugin: classify_url(url) → provider / id
Plugin->>Plugin: build HttpRequest envelope
Plugin->>Host: http_request(envelope)
Host->>API: GET request
API-->>Host: HTTP response (200 / 401/403 / other)
Host-->>Plugin: HttpResponse envelope
Plugin->>Plugin: HttpResponse::into_success_body() / map errors
alt 2xx
Plugin->>Plugin: parse provider response → images/variants
Plugin->>Plugin: dedupe → filter_by_min_resolution → auto_name/select_variant
Plugin-->>Client: JSON ExtractLinksResponse
else 401/403
Plugin-->>Client: PluginError::Private
else other non-2xx
Plugin-->>Client: PluginError::HttpStatus
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested labels
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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: 11
🧹 Nitpick comments (2)
plugins/vortex-mod-gallery/plugin.toml (1)
16-16: Align manifest default with runtime fallback formin_resolution.Line 16 defines a default of
800x600, but runtime fallback currently uses0x0(plugins/vortex-mod-gallery/src/plugin_api.rs, Line 72). Please keep these consistent to avoid config-behavior drift.♻️ Suggested alignment (runtime fallback)
- let min_res = read_config_or("min_resolution", "0x0"); + let min_res = read_config_or("min_resolution", "800x600");🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@plugins/vortex-mod-gallery/plugin.toml` at line 16, The manifest sets min_resolution default to "800x600" but the runtime fallback still uses "0x0"; update the runtime fallback for min_resolution (the code in plugin_api.rs that currently returns "0x0") to return "800x600" so the runtime default matches the plugin.toml manifest and prevents config-behavior drift; ensure the change touches the min_resolution fallback/constant used by any function that parses or supplies the default resolution.plugins/vortex-mod-gallery/src/link.rs (1)
20-41: Reduce duplicated provider type surface.Lines 20-41 mirror
crate::url_matcher::Providerinto a second enum. This works, but it increases long-term churn when provider variants evolve. Prefer a single canonicalProvidertype shared by matcher/filter/output modules.♻️ Refactor direction
- pub enum Provider { ... } // in url_matcher.rs - pub enum Provider { ... } // in link.rs + // Define Provider once in a shared module (e.g., link.rs or provider.rs) + pub enum Provider { Imgur, Reddit, Flickr, Generic } - impl From<crate::url_matcher::Provider> for Provider { ... } + // Import and reuse the shared Provider in url_matcher.rs and filter.rs directly.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@plugins/vortex-mod-gallery/src/link.rs` around lines 20 - 41, Remove the duplicated enum Provider in link.rs and instead re-export the canonical Provider from crate::url_matcher to avoid divergence: replace the local enum and the From impl with a single pub use crate::url_matcher::Provider (or pub use crate::url_matcher::Provider as Provider) so all modules (link.rs, filter.rs, output) share the same type; ensure the canonical Provider in url_matcher has the needed derives/serde attrs (e.g., Debug, Clone, Copy, PartialEq, Eq, Serialize and #[serde(rename_all = "snake_case")]) or move those attributes to that definition if missing, and update any code that referenced the removed From implementation to use the shared type directly.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@CHANGELOG.md`:
- Around line 169-171: Update the CHANGELOG bullets for the SoundCloud plugin,
Vimeo plugin, and Gallery plugin to reflect the actual test counts and artifact
sizes produced by this PR (replace the currently documented numbers that drift
from the implemented outputs); locate the three bullet entries containing
"SoundCloud plugin", "Vimeo plugin", and "Gallery plugin" and edit their text to
match the PR’s stated results (correct unit test counts and any artifact size
metrics) so the changelog aligns with the implementation.
In `@plugins/vortex-mod-gallery/src/providers.rs`:
- Around line 298-303: The FlickrResponse struct must tolerate Flickr error
envelopes: change the photoset field to optional (e.g. photoset:
Option<FlickrPhotoset> with #[serde(default)]) so deserialization won't fail
when Flickr returns {"stat":"fail"} without photoset, then update the code path
that currently calls ParseJson and later accesses photoset to first inspect
FlickrResponse.stat (e.g. check stat == Some("fail") or stat.as_deref() ==
Some("fail")) and return a clear provider error for API/key/private-album
failures before attempting to unwrap or use photoset; ensure any subsequent code
that used photoset handles the Option safely.
- Around line 421-507: parse_generic_html currently calls extract_origin and
resolves relative img src against only scheme://host, causing page-relative
paths to be wrong; change the logic so resolve_url gets the page base
(scheme://host + directory path) instead of just the origin. Update
extract_origin (or add a new helper like extract_base_url) to return the full
base URL up to the page directory for inputs like
https://example.com/gallery/page.html (i.e. https://example.com/gallery), then
adjust resolve_url to use that base when handling paths that do not start with
/, //, or a scheme; keep existing handling for absolute URLs and
protocol-relative URLs. Ensure parse_generic_html calls the new/updated
extractor and passes that base into resolve_url.
In `@plugins/vortex-mod-gallery/src/url_matcher.rs`:
- Line 27: path_only currently only strips the query part using path.split('?')
which leaves URL fragments (e.g. `#fragment`) in the path and causes bad
extractions (like appending .json after a fragment); update the normalization
where path_only is computed so you strip both query and fragment (for example
split on '#' first, then on '?' or vice-versa) and trim_end_matches('/'), and
apply this change at every path_only call site in this module (the path_only
bindings used before regex matching and in the Imgur/Flickr helper extraction
code) so fragments are removed before regex matching and ID extraction.
In `@plugins/vortex-mod-soundcloud/src/lib.rs`:
- Around line 66-70: The helpers advertise playlist support for artist profiles
by matching UrlKind::Artist; remove UrlKind::Artist from the matches so they
only return true for UrlKind::Playlist (i.e., update the match in
handle_supports_playlist to matches!(kind, UrlKind::Playlist) and make the same
change in the other identical helper further down the file), keeping
bool_to_string usage unchanged so the host won't claim artist profile playlist
support until artist pagination is implemented.
In `@plugins/vortex-mod-soundcloud/src/plugin_api.rs`:
- Around line 48-62: The code advertises artist/profile URLs as playlists but
extract_playlist (ResolveResponse::User branch) rejects them at runtime; either
stop advertising artist profiles as playlist-supported by removing
UrlKind::Artist from handle_supports_playlist and ensure_playlist, or implement
the required follow-up pagination call when extract_playlist sees
ResolveResponse::User (call the user-tracks pagination endpoint, build a
playlist response from the paginated results, and return it instead of returning
PluginError::UnsupportedUrl). Update the relevant functions
(handle_supports_playlist, ensure_playlist, and extract_playlist) consistently
so routing and runtime behavior match.
In `@plugins/vortex-mod-soundcloud/src/url_matcher.rs`:
- Around line 58-65: Single-line host-specific classification bug: the pattern
match in segments.as_slice() (in url_matcher.rs) maps single-segment paths
([_user] => UrlKind::Artist) and thus misclassifies on.soundcloud.com short
links as artist pages; update the logic to special-case the on.soundcloud.com
host so single-segment paths are treated as short-track links (UrlKind::Track or
a new ShortLink kind) instead of UrlKind::Artist. Locate the match on
segments.as_slice() and change it to consult the request host (or add a
pre-check for host == "on.soundcloud.com") and return UrlKind::Track for
single-segment paths from that host; keep existing Playlist/Track arms unchanged
and ensure ensure_track in lib.rs still receives Track for on. short links.
In `@plugins/vortex-mod-vimeo/plugin.toml`:
- Around line 16-17: The TOML exposes default_quality but the runtime never
reads it; update the code that selects video quality to use the config or remove
the key: either remove default_quality from plugin.toml, or in
plugins/vortex-mod-vimeo/src/plugin_api.rs read get_config("default_quality")
and apply it where quality is chosen (the function/path that currently decides
download quality—the same code that respects audio_only_preference()); validate
the value against the allowed options ["360p","480p","720p","1080p","2K","4K"],
fall back to a sane default if missing/invalid, and ensure the selected quality
is passed into the download/streaming logic so the config is no longer a no-op.
In `@plugins/vortex-mod-vimeo/src/lib.rs`:
- Around line 171-177: The fallback in pick_cdn is non-deterministic because
hls.cdns.values().next() depends on HashMap iteration order; update pick_cdn
(function pick_cdn and type parser::HlsEntry) to choose a deterministic entry
when default_cdn is None — e.g., select the entry whose key is lexicographically
smallest (or otherwise stable) by iterating hls.cdns.keys() and picking min() or
by using min_by_key on the (key, value) pairs so the fallback HLS URL is stable
across runs.
- Around line 109-115: The function build_single_video_response currently
reconstructs the URL from oembed.video_id (format!("https://vimeo.com/{id}")),
which loses private share tokens and breaks UrlKind::PrivateVideo; update
build_single_video_response to accept and preserve the original source URL
(e.g., add a source_url parameter or read an existing oembed.url if present) and
use that original/private permalink when setting the response URL and
UrlKind::PrivateVideo, then propagate the new source_url argument through all
call sites that invoke build_single_video_response so private links remain
usable in the returned ExtractLinksResponse.
In `@plugins/vortex-mod-vimeo/src/plugin_api.rs`:
- Around line 33-38: extract_links currently calls ensure_vimeo_url which allows
UrlKind::Showcase through and lets showcase URLs reach
build_single_video_response; change the gate to ensure_single_video so non-video
kinds (including Showcase) are rejected early: replace the
ensure_vimeo_url(&url) check in extract_links with a call to
ensure_single_video(&url) (propagate errors the same way using
map_err(error_to_fn_error)) so only true single-video URLs proceed to
fetch_oembed and build_single_video_response.
---
Nitpick comments:
In `@plugins/vortex-mod-gallery/plugin.toml`:
- Line 16: The manifest sets min_resolution default to "800x600" but the runtime
fallback still uses "0x0"; update the runtime fallback for min_resolution (the
code in plugin_api.rs that currently returns "0x0") to return "800x600" so the
runtime default matches the plugin.toml manifest and prevents config-behavior
drift; ensure the change touches the min_resolution fallback/constant used by
any function that parses or supplies the default resolution.
In `@plugins/vortex-mod-gallery/src/link.rs`:
- Around line 20-41: Remove the duplicated enum Provider in link.rs and instead
re-export the canonical Provider from crate::url_matcher to avoid divergence:
replace the local enum and the From impl with a single pub use
crate::url_matcher::Provider (or pub use crate::url_matcher::Provider as
Provider) so all modules (link.rs, filter.rs, output) share the same type;
ensure the canonical Provider in url_matcher has the needed derives/serde attrs
(e.g., Debug, Clone, Copy, PartialEq, Eq, Serialize and #[serde(rename_all =
"snake_case")]) or move those attributes to that definition if missing, and
update any code that referenced the removed From implementation to use the
shared type directly.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 8b56ff4e-a248-49e0-9186-0ea7c32464c5
⛔ Files ignored due to path filters (3)
plugins/vortex-mod-gallery/Cargo.lockis excluded by!**/*.lockplugins/vortex-mod-soundcloud/Cargo.lockis excluded by!**/*.lockplugins/vortex-mod-vimeo/Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (30)
CHANGELOG.mdplugins/vortex-mod-gallery/.cargo/config.tomlplugins/vortex-mod-gallery/Cargo.tomlplugins/vortex-mod-gallery/README.mdplugins/vortex-mod-gallery/plugin.tomlplugins/vortex-mod-gallery/src/error.rsplugins/vortex-mod-gallery/src/filter.rsplugins/vortex-mod-gallery/src/lib.rsplugins/vortex-mod-gallery/src/link.rsplugins/vortex-mod-gallery/src/plugin_api.rsplugins/vortex-mod-gallery/src/providers.rsplugins/vortex-mod-gallery/src/url_matcher.rsplugins/vortex-mod-soundcloud/.cargo/config.tomlplugins/vortex-mod-soundcloud/Cargo.tomlplugins/vortex-mod-soundcloud/README.mdplugins/vortex-mod-soundcloud/plugin.tomlplugins/vortex-mod-soundcloud/src/api.rsplugins/vortex-mod-soundcloud/src/error.rsplugins/vortex-mod-soundcloud/src/lib.rsplugins/vortex-mod-soundcloud/src/plugin_api.rsplugins/vortex-mod-soundcloud/src/url_matcher.rsplugins/vortex-mod-vimeo/.cargo/config.tomlplugins/vortex-mod-vimeo/Cargo.tomlplugins/vortex-mod-vimeo/README.mdplugins/vortex-mod-vimeo/plugin.tomlplugins/vortex-mod-vimeo/src/error.rsplugins/vortex-mod-vimeo/src/lib.rsplugins/vortex-mod-vimeo/src/parser.rsplugins/vortex-mod-vimeo/src/plugin_api.rsplugins/vortex-mod-vimeo/src/url_matcher.rs
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
22 issues found across 33 files
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="plugins/vortex-mod-soundcloud/README.md">
<violation number="1" location="plugins/vortex-mod-soundcloud/README.md:23">
P2: The build command omits `--target wasm32-wasip1`, so it won’t produce the WASM file path referenced in the next steps.</violation>
</file>
<file name="plugins/vortex-mod-gallery/src/filter.rs">
<violation number="1" location="plugins/vortex-mod-gallery/src/filter.rs:47">
P2: `filter_by_min_resolution` keeps partially-known dimensions unconditionally, so images with a known dimension already below the configured minimum are not filtered out.</violation>
</file>
<file name="plugins/vortex-mod-vimeo/src/lib.rs">
<violation number="1" location="plugins/vortex-mod-vimeo/src/lib.rs:77">
P1: `supports_playlist` reports showcase URLs as supported even though `extract_playlist` is unimplemented and always errors.</violation>
<violation number="2" location="plugins/vortex-mod-vimeo/src/lib.rs:114">
P1: Preserve the original Vimeo URL for private-share videos; rebuilding `https://vimeo.com/{id}` drops the private hash token.</violation>
<violation number="3" location="plugins/vortex-mod-vimeo/src/lib.rs:177">
P3: Use a deterministic fallback when `default_cdn` is missing; `HashMap::values().next()` can select different HLS CDNs across runs.</violation>
</file>
<file name="plugins/vortex-mod-vimeo/src/parser.rs">
<violation number="1" location="plugins/vortex-mod-vimeo/src/parser.rs:190">
P2: `extract_player_config_from_html` matches `window.playerConfig` too loosely, so similarly named variables (for example `window.playerConfigVersion`) can be selected first and cause extraction/parsing of the wrong object.</violation>
</file>
<file name="plugins/vortex-mod-soundcloud/src/lib.rs">
<violation number="1" location="plugins/vortex-mod-soundcloud/src/lib.rs:62">
P1: Artist profile URLs are reported as supported even though extraction for `ResolveResponse::User` is not implemented, causing false-positive capability detection and runtime failure.</violation>
<violation number="2" location="plugins/vortex-mod-soundcloud/src/lib.rs:69">
P2: Do not advertise artist profile URLs as playlist-supported until artist pagination is implemented; this currently reports support and then returns `UnsupportedUrl` during playlist extraction.</violation>
</file>
<file name="plugins/vortex-mod-soundcloud/src/url_matcher.rs">
<violation number="1" location="plugins/vortex-mod-soundcloud/src/url_matcher.rs:51">
P2: Fragment identifiers are not removed before path matching, so collection URLs such as `/likes#...` are misclassified as tracks.</violation>
<violation number="2" location="plugins/vortex-mod-soundcloud/src/url_matcher.rs:60">
P2: `on.soundcloud.com/<token>` short links should not be classified as artist profiles; treating any single-segment path as `Artist` misroutes valid short links.</violation>
</file>
<file name="plugins/vortex-mod-soundcloud/plugin.toml">
<violation number="1" location="plugins/vortex-mod-soundcloud/plugin.toml:23">
P2: The `client_id` description claims auto-discovery, but no auto-discovery exists; empty config is passed through as an empty `client_id` value.</violation>
</file>
<file name="plugins/vortex-mod-gallery/src/url_matcher.rs">
<violation number="1" location="plugins/vortex-mod-gallery/src/url_matcher.rs:27">
P1: Strip URL fragments before path matching and permalink construction; otherwise Reddit links with `#...` can generate invalid `.json` URLs and fail extraction.</violation>
<violation number="2" location="plugins/vortex-mod-gallery/src/url_matcher.rs:63">
P2: Avoid always appending `.json` in Reddit permalink extraction; URLs that already end with `.json` become malformed (`.json.json`) and can break fetches.</violation>
</file>
<file name="CHANGELOG.md">
<violation number="1" location="CHANGELOG.md:169">
P3: The changelog test count for SoundCloud is incorrect (listed as 38, but the crate currently has 41 test cases).</violation>
<violation number="2" location="CHANGELOG.md:170">
P3: The changelog test count for Vimeo is incorrect (listed as 43, but the crate currently has 46 test cases).</violation>
</file>
<file name="plugins/vortex-mod-gallery/src/providers.rs">
<violation number="1" location="plugins/vortex-mod-gallery/src/providers.rs:287">
P2: Image URL detection in Reddit fallback fails for `.jpg/.png/...` URLs with query strings or fragments.</violation>
<violation number="2" location="plugins/vortex-mod-gallery/src/providers.rs:300">
P2: Make `photoset` optional in the Flickr response model and check `stat` before accessing it, so API failures are reported as provider errors instead of JSON parse failures.</violation>
<violation number="3" location="plugins/vortex-mod-gallery/src/providers.rs:427">
P2: Resolve relative `<img src>` values against the full page URL instead of just the origin; page-relative paths currently lose their directory context.</violation>
<violation number="4" location="plugins/vortex-mod-gallery/src/providers.rs:483">
P2: Origin parsing does not strip `?`/`#`, producing malformed resolved URLs for base URLs without a path.</violation>
<violation number="5" location="plugins/vortex-mod-gallery/src/providers.rs:499">
P2: Protocol-relative URLs should inherit the page scheme, not always force `https:`.</violation>
</file>
<file name="plugins/vortex-mod-vimeo/src/url_matcher.rs">
<violation number="1" location="plugins/vortex-mod-vimeo/src/url_matcher.rs:47">
P2: Strip URL fragments when matching paths; links like `vimeo.com/<id>#t=...` are currently misclassified as unknown.</violation>
<violation number="2" location="plugins/vortex-mod-vimeo/src/url_matcher.rs:90">
P2: Anchor the showcase/album regex to avoid accepting malformed paths with extra trailing characters.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
Greptile SummaryThis PR adds three new WASM plugins — SoundCloud, Vimeo, and Gallery — each following the existing YouTube plugin template. Logic is kept pure and natively testable, all network I/O is delegated to the host, and WASM-only surface is gated behind Confidence Score: 4/5Safe to merge with awareness of the large-playlist deserialization failure and private-video config gap; remaining items are P2 quality improvements. One P1 finding (Track.title non-optional breaks large SoundCloud playlist parsing) and two meaningful P2 items (Vimeo showcase/extract_playlist contract mismatch; private video config hash not forwarded). Overall implementation quality is high — thorough tests, no panics, correct URL guards, well-documented unsafe blocks — but the stub-track issue is a silent runtime failure on real large playlists that no unit test can catch without live API interaction. plugins/vortex-mod-soundcloud/src/api.rs (Track.title required field), plugins/vortex-mod-vimeo/src/plugin_api.rs (private video hash), plugins/vortex-mod-vimeo/src/lib.rs (supports_playlist vs extract_playlist contract) Important Files Changed
Sequence DiagramsequenceDiagram
participant Host as Plugin Host
participant API as plugin_api.rs (WASM)
participant Logic as Pure Logic (lib.rs / parser.rs)
participant Provider as providers.rs
participant SC_CDN as SoundCloud API v2
participant Vimeo as Vimeo oEmbed / Player Config
participant Gallery as Imgur / Reddit / Flickr API
Note over Host,Gallery: SoundCloud: extract_links / extract_playlist
Host->>API: extract_links(url)
API->>Logic: ensure_soundcloud_url(url)
API->>SC_CDN: http_request(resolve?url=...&client_id=...)
SC_CDN-->>API: ResolveResponse JSON
API->>Logic: response_to_extract_links(resolved)
Logic-->>API: ExtractLinksResponse
API-->>Host: JSON string
Note over Host,Gallery: Vimeo: extract_links + get_media_variants
Host->>API: extract_links(url)
API->>Vimeo: http_request(oEmbed JSON)
Vimeo-->>API: OembedResponse JSON
API->>Logic: build_single_video_response(oembed)
API-->>Host: JSON string
Host->>API: get_media_variants(url)
API->>Logic: extract_video_id(url)
API->>Vimeo: http_request(player.vimeo.com/video/id/config)
Vimeo-->>API: PlayerConfig JSON (or HTML fallback)
API->>Logic: parse_player_config / extract_player_config_from_html
API->>Logic: build_media_variants_response(config)
API-->>Host: JSON string
Note over Host,Gallery: Gallery: extract_links
Host->>API: extract_links(url)
API->>Logic: ensure_recognised_url(url) - Provider
API->>Gallery: http_request(Imgur/Reddit/Flickr API)
Gallery-->>API: API JSON response
API->>Provider: parse_imgur_album / parse_reddit_submission / parse_flickr_photoset
API->>Logic: finalize_links(dedupe + filter + auto_name)
API-->>Host: JSON string
Reviews (1): Last reviewed commit: "feat(plugin): implement SoundCloud, Vime..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
Comment on lines
+129
to
+141
| pub id: u64, | ||
| pub title: String, | ||
| #[serde(default)] | ||
| pub duration: Option<u64>, | ||
| #[serde(default)] | ||
| pub permalink_url: Option<String>, | ||
| #[serde(default)] | ||
| pub artwork_url: Option<String>, | ||
| #[serde(default)] | ||
| pub user: Option<TrackUser>, | ||
| #[serde(default)] | ||
| pub streamable: Option<bool>, | ||
| } |
There was a problem hiding this comment.
Track.title is a required String. The SoundCloud API v2 returns "stub" tracks (only id and kind — no title) for entries beyond the first ~5 in large playlists. Any playlist above that threshold will have its entire /resolve response rejected by serde_json with a missing-field error, silently returning PluginError::ParseJson to the caller.
Making title optional is a safe fix:
Suggested change
| pub id: u64, | |
| pub title: String, | |
| #[serde(default)] | |
| pub duration: Option<u64>, | |
| #[serde(default)] | |
| pub permalink_url: Option<String>, | |
| #[serde(default)] | |
| pub artwork_url: Option<String>, | |
| #[serde(default)] | |
| pub user: Option<TrackUser>, | |
| #[serde(default)] | |
| pub streamable: Option<bool>, | |
| } | |
| #[derive(Debug, Deserialize)] | |
| pub struct Track { | |
| pub id: u64, | |
| #[serde(default)] | |
| pub title: Option<String>, | |
| #[serde(default)] | |
| pub duration: Option<u64>, | |
| #[serde(default)] | |
| pub permalink_url: Option<String>, | |
| #[serde(default)] | |
| pub artwork_url: Option<String>, | |
| #[serde(default)] | |
| pub user: Option<TrackUser>, | |
| #[serde(default)] | |
| pub streamable: Option<bool>, | |
| } |
track_to_link in lib.rs would then need title: track.title.unwrap_or_default(). The existing tests pass title: String in their fixtures, so they remain valid.
plugins/vortex-mod-vimeo/src/lib.rs
Outdated
Comment on lines
+76
to
+78
| pub fn handle_supports_playlist(url: &str) -> String { | ||
| bool_to_string(matches!(url_matcher::classify_url(url), UrlKind::Showcase)) | ||
| } |
There was a problem hiding this comment.
supports_playlist claims Showcase is supported, but extract_playlist always errors
handle_supports_playlist returns "true" for showcase URLs, signalling to the host that it should call extract_playlist. But extract_playlist in plugin_api.rs unconditionally returns PluginError::UnsupportedUrl("showcase extraction is not implemented yet"). Any host that respects supports_playlist will always get a hard error rather than a clean "not supported" signal.
Return "false" until the feature is implemented:
Suggested change
| pub fn handle_supports_playlist(url: &str) -> String { | |
| bool_to_string(matches!(url_matcher::classify_url(url), UrlKind::Showcase)) | |
| } | |
| pub fn handle_supports_playlist(_url: &str) -> String { | |
| // Showcase extraction is not yet implemented; report false until it is. | |
| bool_to_string(false) | |
| } |
Comment on lines
+88
to
+108
| fn fetch_player_config(video_id: &str) -> FnResult<crate::parser::PlayerConfig> { | ||
| let req = build_player_config_request(video_id).map_err(error_to_fn_error)?; | ||
| // SAFETY: identical host-function invariants to `fetch_oembed` | ||
| // above — the host-side symbol, ABI, capability gate, and owned | ||
| // JSON I/O all apply unchanged. See `fetch_oembed` for the full | ||
| // list. | ||
| let raw = unsafe { http_request(req)? }; | ||
| let resp = parse_http_response(&raw).map_err(error_to_fn_error)?; | ||
| let body = resp.into_success_body().map_err(error_to_fn_error)?; | ||
|
|
||
| // Vimeo returns JSON directly for /config. If the body happens to be | ||
| // an HTML page (e.g. geo-blocked fallback) try to extract the config | ||
| // block before giving up. | ||
| match parse_player_config(&body) { | ||
| Ok(cfg) => Ok(cfg), | ||
| Err(_) => { | ||
| let json = extract_player_config_from_html(&body).map_err(error_to_fn_error)?; | ||
| parse_player_config(json).map_err(error_to_fn_error) | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
fetch_player_config calls build_player_config_request(video_id) which constructs https://player.vimeo.com/video/<id>/config — no hash. For private-hash videos (vimeo.com/<id>/<hash>), the Vimeo CDN requires the h=<hash> query parameter on the config endpoint. Without it the request returns 403, surfacing as PluginError::Private — a confusing error even when the caller holds the correct private link.
The url parameter already contains the hash; extract and forward it:
// in url_matcher.rs — extract group 2 from private_video_regex
pub fn extract_private_hash(url: &str) -> Option<String> {
let (_, path) = validate_and_split(url)?;
let path_only = path.split('?').next().unwrap_or("").trim_end_matches('/');
private_video_regex().captures(path_only)
.and_then(|c| c.get(2).map(|m| m.as_str().to_string()))
}
// in build_player_config_request:
pub fn build_player_config_request(video_id: &str, hash: Option<&str>) -> Result<String, PluginError> {
let url = match hash {
Some(h) => format!("https://player.vimeo.com/video/{video_id}/config?h={h}"),
None => format!("https://player.vimeo.com/video/{video_id}/config"),
};
// ...
}
Comment on lines
+98
to
+103
| let response = finalize_links(Provider::Generic, album_id, images, &min_res, auto_name) | ||
| .map_err(error_to_fn_error)?; | ||
| // Override `kind` to signal this came from the generic fallback. | ||
| let json = serde_json::to_string(&response)?; | ||
| Ok(json) | ||
| } |
There was a problem hiding this comment.
kind override never happens
The comment says "Override kind to signal this came from the generic fallback" but no override occurs. finalize_links always sets kind: "gallery" (a &'static str field), so the JSON is identical to a normal gallery response except that provider is "generic". The comment should be corrected to avoid implying missing logic.
Suggested change
| let response = finalize_links(Provider::Generic, album_id, images, &min_res, auto_name) | |
| .map_err(error_to_fn_error)?; | |
| // Override `kind` to signal this came from the generic fallback. | |
| let json = serde_json::to_string(&response)?; | |
| Ok(json) | |
| } | |
| let response = finalize_links(Provider::Generic, album_id, images, &min_res, auto_name) | |
| .map_err(error_to_fn_error)?; | |
| // `provider: "generic"` in the response distinguishes this from | |
| // recognised-provider gallery responses. | |
| Ok(serde_json::to_string(&response)?) |
Comment on lines
+134
to
+149
| fn album_id_for(provider: Provider, url: &str) -> String { | ||
| match provider { | ||
| Provider::Imgur => extract_imgur_id(url).unwrap_or_default(), | ||
| Provider::Flickr => extract_flickr_album_id(url) | ||
| .map(|(_, a)| a) | ||
| .unwrap_or_default(), | ||
| Provider::Reddit => { | ||
| // Use the final path segment as a readable album id. | ||
| url.trim_end_matches('/') | ||
| .rsplit('/') | ||
| .next() | ||
| .unwrap_or("reddit") | ||
| .to_string() | ||
| } | ||
| Provider::Generic => "page".to_string(), | ||
| } |
There was a problem hiding this comment.
album_id uses the title slug, which is unstable and non-unique
For https://reddit.com/r/pics/comments/1abc/my_cat_photo/, album_id_for returns "my_cat_photo" — the human-readable title slug. Two different posts with the same slug produce the same auto-name prefix, causing file collisions. The post ID (1abc) is unique and stable.
Suggested change
| fn album_id_for(provider: Provider, url: &str) -> String { | |
| match provider { | |
| Provider::Imgur => extract_imgur_id(url).unwrap_or_default(), | |
| Provider::Flickr => extract_flickr_album_id(url) | |
| .map(|(_, a)| a) | |
| .unwrap_or_default(), | |
| Provider::Reddit => { | |
| // Use the final path segment as a readable album id. | |
| url.trim_end_matches('/') | |
| .rsplit('/') | |
| .next() | |
| .unwrap_or("reddit") | |
| .to_string() | |
| } | |
| Provider::Generic => "page".to_string(), | |
| } | |
| Provider::Reddit => { | |
| // Use the post ID (segment at index 5 in /r/<sub>/comments/<id>/title) | |
| // rather than the title slug so filenames stay unique across posts. | |
| let path = url.split('?').next().unwrap_or(url).trim_end_matches('/'); | |
| path.split('/').nth(5).unwrap_or("reddit").to_string() | |
| } |
Resolves 25+ findings from CodeRabbit, cubic, and greptile on PR #24. SoundCloud * Strip `#fragment` before path matching and special-case `on.soundcloud.com/<token>` short links so they resolve to Track instead of being misclassified as artist profiles * Remove `UrlKind::Artist` from `handle_can_handle`, `handle_supports_playlist`, and `ensure_soundcloud_url` — extracting artist profiles requires `/users/<id>/tracks` pagination which is not implemented yet, so the host was advertising a capability it could not deliver (false-positive followed by a runtime error) * Update `plugin.toml` `client_id` description: no auto-discovery * `README.md` build command explicitly passes `--target wasm32-wasip1` Vimeo * Strip `#fragment` before path matching (timestamps like `#t=30` are no longer misclassified) * Anchor the showcase/album regex end-of-line so trailing junk is rejected * `handle_can_handle` / `handle_supports_playlist` / `ensure_vimeo_url` no longer accept `UrlKind::Showcase` — showcase extraction is still unimplemented, so advertising support was a false-positive * `build_single_video_response` now takes the original source URL and preserves it verbatim, keeping the `<hash>` token of private share links (`vimeo.com/<id>/<hash>`) instead of rebuilding the URL from `video_id` alone * `pick_cdn` fallback is deterministic: when `default_cdn` is missing, pick the lexicographically smallest CDN key instead of `HashMap::values().next()` * `extract_player_config_from_html` marker now matches on a word boundary, so `window.playerConfigVersion` cannot be mistaken for `window.playerConfig` * `extract_links` uses `ensure_single_video` so showcase URLs are rejected at the entrypoint * `default_quality` config is now honoured — the matching variant is hoisted to the head of the variant list returned by `get_media_variants` Gallery * `classify_url`, `extract_imgur_id`, `extract_reddit_permalink`, and `extract_flickr_album_id` strip both query and fragment before matching * `extract_reddit_permalink` no longer double-appends `.json` when the URL already ends in `.json` * `FlickrResponse.photoset` is now optional and `stat: "fail"` is checked first, so bad API keys / private albums surface as a `PluginError::HttpStatus` carrying the Flickr `code`/`message` instead of a JSON parse error * `parse_generic_html` resolves relative URLs against the **full page directory** (`gallery/page.html` → `gallery/foo.jpg`), not just the origin; protocol-relative URLs inherit the **page scheme** instead of forcing `https:`; `UrlContext::from_page_url` strips `?`/`#` when computing the origin and base directory * `looks_like_image_url` strips query and fragment before checking the extension so Reddit single-image URLs with CDN signing query strings are recognised; `.avif` is now accepted * `filter_by_min_resolution` drops images with a firmly-known single dimension below the threshold (old policy kept any partial-info entry unconditionally) * Canonical `Provider` enum lives in `url_matcher.rs` and is re-exported from `link.rs`; the duplicate `link::Provider` enum is removed along with the `From` conversion * Runtime `min_resolution` fallback (`800x600`) now matches the manifest default, eliminating config-behaviour drift CHANGELOG * Update test counts (41 SoundCloud, 57 Vimeo, 50 Gallery) and WASM sizes (250 KB / 1.12 MB / 1.14 MB) to match the actual PR outputs Test totals: 51 SoundCloud + 57 Vimeo + 50 Gallery = 158 native unit tests (up from 133 in the initial PR commit). Clippy `-D warnings` clean on both `wasm32-wasip1` and `x86_64-unknown-linux-gnu` for all three crates.
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Nitpick comments (3)
plugins/vortex-mod-vimeo/src/lib.rs (2)
79-86: Consider removing unusedclassify_urlcall.The result of
url_matcher::classify_url(url)is explicitly discarded and has no effect on the return value. Since the function unconditionally returns"false", this call appears to be dead code.♻️ Proposed simplification
pub fn handle_supports_playlist(url: &str) -> String { // Same rationale as `handle_can_handle`: showcase enumeration // requires an access-token endpoint that is not wired in this MVP. // Report `false` so the host never routes showcase URLs to a // handler that can only fail. - let _ = url_matcher::classify_url(url); + let _ = url; // Silence unused parameter warning until showcase is implemented. bool_to_string(false) }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@plugins/vortex-mod-vimeo/src/lib.rs` around lines 79 - 86, The call to url_matcher::classify_url(url) inside handle_supports_playlist is dead code because its result is discarded and the function always returns bool_to_string(false); remove the unused call (or if intended, use its result to influence the return) so handle_supports_playlist only returns bool_to_string(false) without calling url_matcher::classify_url, and ensure you keep the function signature and comment rationale intact.
96-110: Near-duplicate validation functions.
ensure_vimeo_urlandensure_single_videoare functionally identical sinceUrlKindonly has four variants (Video,PrivateVideo,Showcase,Unknown). The explicitShowcase | Unknownpattern vs. the wildcard_produces the same behavior.If the intent is to keep separate functions for semantic clarity at call sites, consider adding a brief doc comment to each explaining the distinction. Otherwise, one could be removed or implemented in terms of the other.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@plugins/vortex-mod-vimeo/src/lib.rs` around lines 96 - 110, These two functions (ensure_vimeo_url and ensure_single_video) are identical in behavior because UrlKind has only Video, PrivateVideo, Showcase, Unknown; either document their semantic difference or consolidate: remove one and call the remaining function from its former callers, or implement ensure_single_video by delegating to ensure_vimeo_url (or vice versa) and returning its Result; locate usages of url_matcher::classify_url and PluginError::UnsupportedUrl to update callers accordingly and add brief doc comments to the kept function describing the intended semantic distinction if you keep both.plugins/vortex-mod-vimeo/src/plugin_api.rs (1)
159-173: Consider documenting accepted truthy values.The
audio_only_preferencefunction accepts"true","1", and"yes"(case-insensitive), which is reasonable for user-facing config. If this convention is consistent across plugins, a brief inline comment or shared constant could help maintainability.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@plugins/vortex-mod-vimeo/src/plugin_api.rs` around lines 159 - 173, The audio_only_preference() function accepts case-insensitive truthy strings "true", "1", and "yes" but doesn't document them; update the function (or a shared constant) to explicitly list these accepted values and why (e.g., case-insensitive user config), and replace the inline literal match with a reference to that named constant (e.g., TRUTHY_VALUES) so the convention is discoverable and reusable across plugins; mention get_config("extract_audio_only") in the comment so readers know which config key uses this convention.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@plugins/vortex-mod-gallery/src/filter.rs`:
- Around line 87-95: The function guess_ext_from_url currently only strips the
query string and can leave a fragment (e.g. "image.avif#foo") causing ext
validation to fail; update guess_ext_from_url to also remove any fragment by
splitting on '#' (or otherwise truncating at the first '#') before locating the
last '.' and extracting ext, then proceed with the existing length and
alphanumeric checks so files like "image.avif#fragment" correctly yield "avif".
In `@plugins/vortex-mod-gallery/src/plugin_api.rs`:
- Around line 146-152: The Reddit branch currently derives album_id directly
from the raw url, which can include query params or fragments; instead call the
existing extract_reddit_permalink(url) to get the canonical path and then take
its final segment (trim_end_matches('/'),
rsplit().next().unwrap_or("reddit").to_string()) from that canonicalized
permalink; update the Provider::Reddit arm to use the extract_reddit_permalink
result when constructing the album id.
In `@plugins/vortex-mod-gallery/src/url_matcher.rs`:
- Around line 115-127: The provider regexes (imgur_regex, reddit_regex,
flickr_regex) only assert prefixes and therefore accept malformed paths like
"/gallery/abc-typo" or "/albums/123junk"; update each regex to anchor the
captured segment to segment boundaries by requiring either end-of-string or a
slash immediately after the capture (e.g. change the imgur pattern to require
(?:$|/) after the ID, similarly append (?:$|/) after the reddit post id and
after the flickr album id) so the extractor only matches whole path segments and
rejects typos.
In `@plugins/vortex-mod-soundcloud/src/lib.rs`:
- Around line 180-183: The error returned from the ResolveResponse::User arm
incorrectly tells callers to "call extract_playlist" even though ensure_playlist
(and UrlKind::Playlist) still rejects artist URLs; update the
PluginError::UnsupportedUrl message in the ResolveResponse::User branch to a
plain "artist profiles are not supported yet" (or similarly neutral text)
instead of recommending extract_playlist, or alternatively implement
artist-profile handling in extract_playlist/ensure_playlist before leaving that
guidance; reference ResolveResponse::User, PluginError::UnsupportedUrl,
extract_playlist, ensure_playlist, and UrlKind::Playlist when making the change.
- Around line 138-149: The upgrade_artwork function currently uses
url.find("-large") which matches the first occurrence; change this to
url.rfind("-large") so it searches from the end and updates only the trailing
artwork size suffix. Keep the existing boundary check (compute after =
url.as_bytes().get(idx + "-large".len()).copied().unwrap_or(0) and
matches!(after, 0 | b'.' | b'/' | b'?' | b'#')) and the replacement logic
(format!("{}-t500x500{}", &url[..idx], &url[idx + "-large".len()..])), and
return the original url when rfind returns None. Ensure these changes are made
inside the upgrade_artwork function.
---
Nitpick comments:
In `@plugins/vortex-mod-vimeo/src/lib.rs`:
- Around line 79-86: The call to url_matcher::classify_url(url) inside
handle_supports_playlist is dead code because its result is discarded and the
function always returns bool_to_string(false); remove the unused call (or if
intended, use its result to influence the return) so handle_supports_playlist
only returns bool_to_string(false) without calling url_matcher::classify_url,
and ensure you keep the function signature and comment rationale intact.
- Around line 96-110: These two functions (ensure_vimeo_url and
ensure_single_video) are identical in behavior because UrlKind has only Video,
PrivateVideo, Showcase, Unknown; either document their semantic difference or
consolidate: remove one and call the remaining function from its former callers,
or implement ensure_single_video by delegating to ensure_vimeo_url (or vice
versa) and returning its Result; locate usages of url_matcher::classify_url and
PluginError::UnsupportedUrl to update callers accordingly and add brief doc
comments to the kept function describing the intended semantic distinction if
you keep both.
In `@plugins/vortex-mod-vimeo/src/plugin_api.rs`:
- Around line 159-173: The audio_only_preference() function accepts
case-insensitive truthy strings "true", "1", and "yes" but doesn't document
them; update the function (or a shared constant) to explicitly list these
accepted values and why (e.g., case-insensitive user config), and replace the
inline literal match with a reference to that named constant (e.g.,
TRUTHY_VALUES) so the convention is discoverable and reusable across plugins;
mention get_config("extract_audio_only") in the comment so readers know which
config key uses this convention.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 9de76a60-7dad-4957-a941-0f06ca44e487
📒 Files selected for processing (16)
CHANGELOG.mdplugins/vortex-mod-gallery/src/filter.rsplugins/vortex-mod-gallery/src/lib.rsplugins/vortex-mod-gallery/src/link.rsplugins/vortex-mod-gallery/src/plugin_api.rsplugins/vortex-mod-gallery/src/providers.rsplugins/vortex-mod-gallery/src/url_matcher.rsplugins/vortex-mod-soundcloud/README.mdplugins/vortex-mod-soundcloud/plugin.tomlplugins/vortex-mod-soundcloud/src/lib.rsplugins/vortex-mod-soundcloud/src/url_matcher.rsplugins/vortex-mod-vimeo/plugin.tomlplugins/vortex-mod-vimeo/src/lib.rsplugins/vortex-mod-vimeo/src/parser.rsplugins/vortex-mod-vimeo/src/plugin_api.rsplugins/vortex-mod-vimeo/src/url_matcher.rs
✅ Files skipped from review due to trivial changes (4)
- plugins/vortex-mod-gallery/src/link.rs
- plugins/vortex-mod-soundcloud/README.md
- plugins/vortex-mod-vimeo/plugin.toml
- plugins/vortex-mod-soundcloud/plugin.toml
🚧 Files skipped from review as they are similar to previous changes (6)
- CHANGELOG.md
- plugins/vortex-mod-soundcloud/src/url_matcher.rs
- plugins/vortex-mod-gallery/src/lib.rs
- plugins/vortex-mod-vimeo/src/url_matcher.rs
- plugins/vortex-mod-vimeo/src/parser.rs
- plugins/vortex-mod-gallery/src/providers.rs
There was a problem hiding this comment.
2 issues found across 16 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="plugins/vortex-mod-vimeo/src/parser.rs">
<violation number="1" location="plugins/vortex-mod-vimeo/src/parser.rs:256">
P2: The new word-boundary check misses `$` as an identifier character, so similarly named variables like `window.playerConfig$...` can still be misdetected as the real marker.</violation>
</file>
<file name="plugins/vortex-mod-gallery/src/providers.rs">
<violation number="1" location="plugins/vortex-mod-gallery/src/providers.rs:551">
P2: Absolute image URLs with uppercase scheme are mis-resolved as relative paths due to case-sensitive `http(s)` detection.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
Resolves 10 additional findings from CodeRabbit and cubic on PR #24 after the round-2 commit. SoundCloud * `upgrade_artwork` now uses `rfind("-large")` instead of `find` so that a URL whose earlier path segment happens to contain `-large` (e.g. `/user/too-large-a-track/artworks-999-large.jpg`) is still upgraded only at the trailing artwork-size suffix * The `ResolveResponse::User` error message in `response_to_extract_links` no longer tells callers to "call extract_playlist" — `ensure_playlist` rejects artist URLs too, so the guidance was misleading. Now: "artist profiles are not supported yet — artist pagination is not implemented" Vimeo * `handle_supports_playlist` signature is now `(_url: &str)` and the dead `classify_url(url)` call is removed. The function unconditionally returns `false`; the URL is intentionally ignored until showcase extraction is wired up * `ensure_vimeo_url` now delegates to `ensure_single_video` and both functions carry doc comments explaining the intended semantic distinction (top-level routing vs. single-video handlers) so a future reviewer can tell why both names exist even though they are currently functionally equivalent * `audio_only_preference` now reads truthy values through a shared `TRUTHY_VALUES` constant and a named `is_truthy` helper, with a doc comment stating the accepted values and the config key they apply to * `find_at_word_boundary` in the HTML player-config extractor now rejects `$` as an identifier-continuation character (JavaScript allows `$` in identifiers, so `window.playerConfig$legacy` must not satisfy the word boundary for `window.playerConfig`). The check is factored into `is_js_ident_continue` with a documenting comment Gallery * `guess_ext_from_url` now strips `#fragment` before locating the last `.`, so inputs like `image.avif#foo` correctly yield `"avif"` instead of failing the alphanumeric check * `album_id_for` for the Reddit provider now derives the album id from the canonical permalink returned by `extract_reddit_permalink` (which has already stripped query/fragment and appended `.json`), trimming the `.json` suffix and taking the final path segment. The raw URL path is no longer used, so `?utm_source=...` and `#comment` no longer leak into the auto-generated filename * `imgur_regex`, `reddit_regex`, and `flickr_regex` now anchor their captured segments to a segment boundary (`(?:$|/)`) so malformed paths like `/a/abc-typo`, `/r/pics/comments/1abcjunk-extra`, and `/photos/bob/albums/123junk` are rejected instead of producing a partial match that the extractor would happily use * Scheme detection in `UrlContext::resolve` and `is_http_url` is now case-insensitive via `is_absolute_http_url` — legacy HTML authors sometimes write `HTTP://` / `HTTPS://`, and a case-sensitive check would route those into the relative-URL branch and produce malformed output Test totals: 52 SoundCloud + 58 Vimeo + 57 Gallery = 167 native unit tests (up from 158). Clippy `-D warnings` clean on both `wasm32-wasip1` and `x86_64-unknown-linux-gnu` for all three crates.
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (1)
plugins/vortex-mod-soundcloud/src/lib.rs (1)
391-400: Rename this test to reflect the current contract.
response_to_extract_links_user_routes_to_extract_playliststill describes the pre-fix behavior, but theUserbranch now rejects artist profiles. Renaming it will keep the test suite self-explanatory.Suggested rename
- fn response_to_extract_links_user_routes_to_extract_playlist() { + fn response_to_extract_links_user_is_rejected_until_pagination_is_implemented() {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@plugins/vortex-mod-soundcloud/src/lib.rs` around lines 391 - 400, The test named response_to_extract_links_user_routes_to_extract_playlist no longer reflects behavior—since response_to_extract_links(User) now rejects artist profiles—rename the test to something like response_to_extract_links_user_rejects_artist_profiles (or response_to_extract_links_user_routes_to_unsupported_url) and update the function declaration to that new identifier; ensure the test still calls response_to_extract_links with a User and asserts matches!(err, PluginError::UnsupportedUrl(_)), and update any references or documentation to the old test name.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@plugins/vortex-mod-gallery/src/providers.rs`:
- Around line 332-345: The FlickrPhoto struct currently only captures
width_o/height_o while the code may emit url_l; update the FlickrPhoto struct to
also include width_l and height_l (serde defaulted Option<...> like the others),
update the Flickr API request to ask for the corresponding size fields (include
url_l,width_l,height_l in the fields/query), and modify the selection logic
(where you build the output URL/dimensions) to choose width/height matching
whichever URL variant you emit (use width_l/height_l when emitting url_l,
otherwise width_o/height_o) so images aren’t dropped by the min_resolution check
in plugin_api.rs.
- Around line 150-160: Reddit gallery ordering is broken because you iterate
media_metadata (a HashMap) and sort by URL; instead parse and use
RedditPost.gallery_data.items (each with media_id) to preserve the post's
sequence: update the RedditPost struct to include an optional gallery_data field
(with items Vec<{media_id: String}>), then in the gallery handling code walk
gallery_data.items in order, lookup each media_id in media_metadata to build the
ordered list of URLs/filenames; only if gallery_data is missing fall back to the
existing media_metadata URL-based deterministic sort. Ensure you reference
RedditPost.gallery_data, RedditPost.media_metadata and the media_id keys when
implementing this change.
In `@plugins/vortex-mod-soundcloud/src/lib.rs`:
- Around line 91-110: The helpers ensure_soundcloud_url, ensure_track, and
ensure_playlist currently classify valid SoundCloud URLs but always return
PluginError::UnsupportedUrl(...) for recognized-but-wrong kinds; update them to
return a kind-specific error instead (either add a new PluginError variant like
UnsupportedUrlKind(UrlKind, String) or extend UnsupportedUrl to include the
detected UrlKind) and change the return paths in ensure_soundcloud_url,
ensure_track, and ensure_playlist to construct that new/extended error with the
detected UrlKind and original URL string so callers see “unsupported resource
kind” rather than “unrecognized URL.”
In `@plugins/vortex-mod-vimeo/src/parser.rs`:
- Around line 169-170: parse_player_config currently calls serde_json::from_str
which rejects JS object literals with single-quoted strings; update
parse_player_config to handle that fallback by attempting to normalize
JavaScript-style object literals into valid JSON before parsing: try
serde_json::from_str(raw) first, and if it fails, run a safe normalization on
the extracted payload (used by extract_player_config_from_html) to convert
single-quoted string tokens to double-quoted JSON strings while preserving
escapes and not touching already-quoted content, then retry serde_json::from_str
on the normalized string; reference parse_player_config and the extractor
extract_player_config_from_html so tests exercising extraction→parsing will
succeed.
---
Nitpick comments:
In `@plugins/vortex-mod-soundcloud/src/lib.rs`:
- Around line 391-400: The test named
response_to_extract_links_user_routes_to_extract_playlist no longer reflects
behavior—since response_to_extract_links(User) now rejects artist
profiles—rename the test to something like
response_to_extract_links_user_rejects_artist_profiles (or
response_to_extract_links_user_routes_to_unsupported_url) and update the
function declaration to that new identifier; ensure the test still calls
response_to_extract_links with a User and asserts matches!(err,
PluginError::UnsupportedUrl(_)), and update any references or documentation to
the old test name.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 63b5310e-ca07-46c3-bc4d-e3e7199f6c52
📒 Files selected for processing (8)
plugins/vortex-mod-gallery/src/filter.rsplugins/vortex-mod-gallery/src/plugin_api.rsplugins/vortex-mod-gallery/src/providers.rsplugins/vortex-mod-gallery/src/url_matcher.rsplugins/vortex-mod-soundcloud/src/lib.rsplugins/vortex-mod-vimeo/src/lib.rsplugins/vortex-mod-vimeo/src/parser.rsplugins/vortex-mod-vimeo/src/plugin_api.rs
🚧 Files skipped from review as they are similar to previous changes (3)
- plugins/vortex-mod-gallery/src/filter.rs
- plugins/vortex-mod-gallery/src/url_matcher.rs
- plugins/vortex-mod-vimeo/src/plugin_api.rs
There was a problem hiding this comment.
1 issue found across 8 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="plugins/vortex-mod-soundcloud/src/lib.rs">
<violation number="1" location="plugins/vortex-mod-soundcloud/src/lib.rs:146">
P2: Using `rfind` across the full URL can rewrite `-large` inside query/fragment data instead of the artwork size suffix in the path.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
Resolves 6 additional findings from CodeRabbit and cubic on PR #24 after the round-3 commit. SoundCloud * `upgrade_artwork` now splits the URL into `(path, query_fragment)` before scanning for `-large`. `rfind` is still used inside the path, so `/user/too-large-a-track/artworks-999-large.jpg` is still upgraded correctly, but a `?ref=-large-thing` query string can no longer be mistaken for an artwork size suffix (cubic finding) * New `PluginError::UnsupportedResourceKind { kind, url }` variant distinguishes "valid SoundCloud URL of the wrong kind for this operation" from "not a SoundCloud URL at all". `ensure_soundcloud_url`, `ensure_track`, and `ensure_playlist` now return the kind-specific variant when the URL is recognised but the kind mismatches, and the plain `UnsupportedUrl` variant only when the classifier returns `UrlKind::Unknown`. Callers can now give a targeted error message ("playlist URL not valid here") instead of a generic "not a SoundCloud URL" message * `response_to_extract_links_user_routes_to_extract_playlist` renamed to `response_to_extract_links_user_rejects_artist_profile_until_pagination` and the assertion now also verifies that the error message does NOT suggest `extract_playlist` (which would just fail again) Vimeo * `parse_player_config` now accepts JavaScript object literals with single-quoted strings. The happy path still calls `serde_json::from_str` directly (strict JSON from `/config` is unchanged), but when strict parsing fails the new `js_object_literal_to_json` helper rewrites unescaped `'` outside `"`-delimited strings into `"`, preserving escape sequences and escaping any literal `"` that appears inside the now-rewritten string. Re-parse with serde and return the result. A direct-JSON input passes through untouched * Fix clippy `unnecessary_cast` warning: `'\\' as char` → `'\\'` Gallery * `FlickrPhoto` struct now also captures `width_l` / `height_l`, and `parse_flickr_photoset` reads the dimensions that match the URL size it emits — `width_o`/`height_o` when it uses `url_o`, `width_l`/`height_l` when it falls back to `url_l`. Without this, `url_l`-only images were being filtered wrongly by `filter_by_min_resolution` because the dimensions it checked came from the missing `width_o`/`height_o` fields * `build_flickr_request` now asks Flickr for all six extras (`url_o`, `width_o`, `height_o`, `url_l`, `width_l`, `height_l`) so the parser actually has the data to use * `RedditPost` now models `gallery_data.items` (an ordered array of `media_id` refs). `parse_reddit_submission` walks this array first and preserves the submission's image sequence; only when `gallery_data` is missing does it fall back to the deterministic URL-sorted enumeration of `media_metadata`. This fixes gallery ordering for native Reddit galleries, which are the common case Test totals: 57 SoundCloud + 62 Vimeo + 61 Gallery = 180 native unit tests (up from 167). Clippy `-D warnings` clean on both `wasm32-wasip1` and `x86_64-unknown-linux-gnu` for all three crates.
There was a problem hiding this comment.
🧹 Nitpick comments (1)
plugins/vortex-mod-gallery/src/providers.rs (1)
523-530: Consider usingexpect()for compile-time constant regex.While this regex is a compile-time constant and will always compile successfully, the PR objectives state "no
.unwrap()outside tests." Using.expect()with a clear message would document the invariant and align with the stated policy.Proposed change
fn img_src_regex() -> &'static Regex { static R: OnceLock<Regex> = OnceLock::new(); R.get_or_init(|| { // Match <img ... src="..." ...> and capture the src value. // Uses [^>]* rather than .*? to avoid backtracking on large pages. - Regex::new(r#"(?i)<img\b[^>]*\bsrc\s*=\s*["']([^"']+)["']"#).unwrap() + Regex::new(r#"(?i)<img\b[^>]*\bsrc\s*=\s*["']([^"']+)["']"#) + .expect("img_src_regex is a compile-time constant") }) }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@plugins/vortex-mod-gallery/src/providers.rs` around lines 523 - 530, In img_src_regex(), replace the Regex::new(...).unwrap() call with Regex::new(...).expect("compile-time constant regex in img_src_regex failed") so the invariant is documented and we avoid using unwrap() outside tests; update the expect message to clearly name img_src_regex and that the regex is expected to be valid at compile-time.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@plugins/vortex-mod-gallery/src/providers.rs`:
- Around line 523-530: In img_src_regex(), replace the Regex::new(...).unwrap()
call with Regex::new(...).expect("compile-time constant regex in img_src_regex
failed") so the invariant is documented and we avoid using unwrap() outside
tests; update the expect message to clearly name img_src_regex and that the
regex is expected to be valid at compile-time.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: ac6c609d-841c-4ac5-9d5c-669806c9d19c
📒 Files selected for processing (4)
plugins/vortex-mod-gallery/src/providers.rsplugins/vortex-mod-soundcloud/src/error.rsplugins/vortex-mod-soundcloud/src/lib.rsplugins/vortex-mod-vimeo/src/parser.rs
🚧 Files skipped from review as they are similar to previous changes (1)
- plugins/vortex-mod-vimeo/src/parser.rs
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="plugins/vortex-mod-vimeo/src/parser.rs">
<violation number="1" location="plugins/vortex-mod-vimeo/src/parser.rs:219">
P2: `js_object_literal_to_json` corrupts UTF-8 by iterating bytes and casting to `char`, which can mangle non-ASCII metadata in fallback player-config parsing.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
Resolves 2 findings from CodeRabbit and cubic on PR #24 after the round-4 commit, plus consistent migration of every OnceLock regex init to `.expect` so the crate-wide "no `.unwrap()` outside tests" policy is honoured everywhere, not just at the one site the reviewer happened to notice. Vimeo * `js_object_literal_to_json` now iterates `input.chars()` instead of `input.bytes()`, so non-ASCII metadata embedded in the player config — titles with accented characters, em-dashes, emoji, CJK, etc. — round-trips through the single-quoted → double-quoted rewrite without corruption. The old byte-level loop was pushing each UTF-8 continuation byte as a separate `char`, mangling multi-byte sequences. Added regression tests covering `"Éclair — intro 🎬"` both as pure JSON input and as a JS object literal piped through `parse_player_config` end-to-end (cubic finding) * `video_regex`, `private_video_regex`, `showcase_or_album_regex`, and `player_video_regex` now call `.expect(..)` with a message naming the regex and the compile-time-constant invariant Gallery * `img_src_regex` now calls `.expect("img_src_regex: compile-time constant regex must compile")` (CodeRabbit finding) * Same migration applied to `imgur_regex`, `reddit_regex`, and `flickr_regex` for consistency — all seven regex inits across the three plugin crates now document the compile-time-constant invariant and honour the crate-wide "no `.unwrap()` outside tests" policy Test totals: 57 SoundCloud + 65 Vimeo + 61 Gallery = 183 native unit tests (up from 180). Clippy `-D warnings` clean on both `wasm32-wasip1` and `x86_64-unknown-linux-gnu` for all three crates.
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
plugins/vortex-mod-gallery/src/url_matcher.rs (1)
151-169: Minor edge case: IPv6 literal hosts are not handled.The port-stripping logic at line 162-163 splits on
:, which would incorrectly truncate IPv6 literal hosts like[::1]:8080to[. This is unlikely to occur in practice for gallery URLs, but worth noting.Suggested fix for IPv6 support
- let host = authority_no_user - .split(':') - .next() - .unwrap_or(authority_no_user); + let host = if authority_no_user.starts_with('[') { + // IPv6 literal: [::1] or [::1]:port + authority_no_user + .find(']') + .map(|i| &authority_no_user[..=i]) + .unwrap_or(authority_no_user) + } else { + authority_no_user + .split(':') + .next() + .unwrap_or(authority_no_user) + };🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@plugins/vortex-mod-gallery/src/url_matcher.rs` around lines 151 - 169, The validate_and_split function mishandles IPv6 literal hosts because it naively splits authority on ':'; update validate_and_split to detect bracketed IPv6 (authority_no_user starts_with '[') and extract the host as the substring between '[' and ']' (and strip the optional port after the closing ']'), otherwise keep the existing behavior of taking text before the first ':' for host; ensure host is lowercased and non-empty before returning. Reference validate_and_split and the variables authority_no_user, host, path_and_query when making the change.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@plugins/vortex-mod-vimeo/src/parser.rs`:
- Around line 289-358: The current marker detection (using CANONICAL/FALLBACK
and find_at_word_boundary) can match non-assignment occurrences; change the
logic so the match requires a left identifier boundary and then verify an actual
assignment operator before extracting the `{...}`: update find_at_word_boundary
(or add a new helper) to check the byte before the needle is not an identifier
char (so both left and right boundaries), then after computing brace_start
ensure there is an `=` between the needle end and the found `{` (skip
whitespace/comments) and only proceed if `=` is present; use the existing
symbols CANONICAL, FALLBACK, find_at_word_boundary, brace_start and the
brace-finding loop to keep the rest unchanged.
---
Nitpick comments:
In `@plugins/vortex-mod-gallery/src/url_matcher.rs`:
- Around line 151-169: The validate_and_split function mishandles IPv6 literal
hosts because it naively splits authority on ':'; update validate_and_split to
detect bracketed IPv6 (authority_no_user starts_with '[') and extract the host
as the substring between '[' and ']' (and strip the optional port after the
closing ']'), otherwise keep the existing behavior of taking text before the
first ':' for host; ensure host is lowercased and non-empty before returning.
Reference validate_and_split and the variables authority_no_user, host,
path_and_query when making the change.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 29692d2c-371e-489d-a24b-6e2c23ce0726
📒 Files selected for processing (4)
plugins/vortex-mod-gallery/src/providers.rsplugins/vortex-mod-gallery/src/url_matcher.rsplugins/vortex-mod-vimeo/src/parser.rsplugins/vortex-mod-vimeo/src/url_matcher.rs
🚧 Files skipped from review as they are similar to previous changes (1)
- plugins/vortex-mod-vimeo/src/url_matcher.rs
Resolves 2 findings from CodeRabbit on PR #24 after the round-5 commit. Vimeo * `extract_player_config_from_html` now uses a new helper `find_assignment_marker` that checks **both** left and right identifier boundaries (so `mywindow.playerConfig` and `.playerConfig$` no longer match `window.playerConfig` / `playerConfig =`) and, for the bare `CANONICAL` marker, verifies that the gap between the needle end and the next `{` contains an `=` operator. This rejects non-assignment references such as `console.log(window.playerConfig)` that happen to appear before the real assignment in the HTML. The `FALLBACK` needle already contains `=`, so the gap scan is skipped via a `RequireAssignment::No` flag. Added regression tests for the left-boundary violation, the non-assignment reference, and the fallback happy path Gallery / SoundCloud / Vimeo url_matcher * New helper `extract_host` in each `validate_and_split` recognises bracketed IPv6 literal hosts (`[::1]`, `[2001:db8::1]:443`) and keeps the brackets so downstream host-allowlist matches still behave as expected. Plain hosts and IPv4 continue to use the existing split-on-`:` logic. Lone `[` with no closing `]` returns `None`. Applied to all three plugins for consistency, since they share the same `validate_and_split` shape. Added unit tests in the gallery plugin covering plain, IPv4, IPv6 with and without port, malformed, and empty cases Test totals: 57 SoundCloud + 68 Vimeo + 62 Gallery = 187 native unit tests (up from 183). Clippy `-D warnings` clean on both `wasm32-wasip1` and `x86_64-unknown-linux-gnu` for all three crates.
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="plugins/vortex-mod-vimeo/src/parser.rs">
<violation number="1" location="plugins/vortex-mod-vimeo/src/parser.rs:402">
P1: The new marker check can misclassify non-assignment references as assignments because it accepts any `=` in the gap, including `==`/`===` comparisons.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
Resolves 1 P1 finding from cubic on PR #24 after the round-6 commit. Vimeo * `find_assignment_marker`'s gap check previously used `gap.contains('=')` which also matched `==`, `===`, `!=`, `!==`, `<=`, `>=`, and `=>`. A JavaScript snippet like `if (window.playerConfig === null) { legacy() }` appearing before the real assignment would therefore be picked up as an assignment site, sending the brace-balancing scanner into the comparison branch instead of the config object. * New helper `gap_contains_assignment` scans the gap byte-by-byte and only accepts an `=` whose preceding byte is not `=`, `!`, `<`, or `>` and whose following byte is not `=` or `>`. This rejects every comparison operator and the arrow-function `=>` while still accepting plain `=` with or without surrounding whitespace * Added regression tests for `===`, `==`, `!==`, `=>` before the real assignment, plus direct unit tests for `gap_contains_assignment` covering every shape Test totals: 57 SoundCloud + 73 Vimeo + 62 Gallery = 192 native unit tests (up from 187). Clippy `-D warnings` clean on both `wasm32-wasip1` and `x86_64-unknown-linux-gnu` for all three crates.
There was a problem hiding this comment.
1 issue found across 1 file (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="plugins/vortex-mod-vimeo/src/parser.rs">
<violation number="1" location="plugins/vortex-mod-vimeo/src/parser.rs:434">
P2: `gap_contains_assignment` can misclassify `=` inside quoted strings as assignment, causing false marker matches and extraction of the wrong `{...}` block.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
Resolves 1 P2 finding from cubic on PR #24 after the round-7 commit. Vimeo * `gap_contains_assignment` now tracks JavaScript string-literal state (`"`, `'`, `\` escape) while scanning for a bare `=` in the gap between a marker needle and the next `{`. Previously, a code snippet like `const msg = "debug: window.playerConfig = bogus";` before the real assignment would be picked up as an assignment site because the `=` inside the string literal looked like a plain assignment operator. The new scanner skips bytes that are inside a `"`-delimited or `'`-delimited string, matching the policy the rest of this module already uses for HTML-embedded JS. * Added regression tests for: `=` inside double-quoted string, `=` inside single-quoted string, escaped quote inside string, real `=` appearing *after* a string literal, and the end-to-end `extract_player_config_from_html` decoy case where the marker appears inside a string literal before the real assignment. Test totals: 57 SoundCloud + 76 Vimeo + 62 Gallery = 195 native unit tests (up from 192). Clippy `-D warnings` clean on both `wasm32-wasip1` and `x86_64-unknown-linux-gnu` for all three crates.
There was a problem hiding this comment.
1 issue found across 1 file (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="plugins/vortex-mod-vimeo/src/parser.rs">
<violation number="1" location="plugins/vortex-mod-vimeo/src/parser.rs:445">
P1: `gap_contains_assignment` resets quote state at the start of `gap`, so markers that occur inside an already-open string can still be treated as assignments and cause extraction to lock onto an unrelated `{...}` block.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
Resolves 1 P1 finding from cubic on PR #24 after the round-8 commit. Vimeo * `find_assignment_marker` is now a **single-pass** scanner that tracks JavaScript string state (`"`, `'`, `\` escape) from position 0 of the haystack. A needle occurrence inside an already-open string literal is skipped because the scanner knows the string was opened before the needle started. This eliminates the class of bugs where `gap_contains_assignment` reset quote state at the start of a substring and could not tell it was already inside a string * `gap_has_assignment_then_brace` replaces `gap_contains_assignment` for the live code path: it receives a `bytes` slice and a `start` offset that is guaranteed outside any string by the caller, then walks forward tracking strings until it finds `{` (outside strings). If a bare `=` (outside strings) appears before the `{`, it returns `true`. The old standalone `gap_contains_assignment` is kept behind `#[cfg(test)]` as a convenience wrapper that appends a `{` sentinel so existing unit tests still pass * New helper `find_brace_outside_strings` replaces the plain `rest.find('{')` in `extract_player_config_from_html` so the balanced-brace scanner starts at the first `{` that is genuinely outside a string literal * Added regression test: `window.playerConfig = {bad: true}` appears inside a string that was already open before the marker — the single-pass scanner correctly skips it and extracts the real assignment below Test totals: 57 SoundCloud + 77 Vimeo + 62 Gallery = 196 native unit tests (up from 195). Clippy `-D warnings` clean on both `wasm32-wasip1` and `x86_64-unknown-linux-gnu`.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Closes task 25 of the MVP spec. Adds three new WASM plugins under
plugins/, each following the same layout as the already-mergedvortex-mod-youtubetemplate: pure logic in sibling modules, WASM-onlyplugin_api.rsgated behindcfg(target_family = \"wasm\"), and all network I/O delegated to the hosthttp_requestcapability./resolveAPI v2 client with tagged enum dispatch (Track / Playlist / User / Unknown), URL router coveringsets,likes,tracks,albums,m./on.hosts, artwork upgrade from-largeto-t500x500(handles.ext, no-ext, and query-string variants),client_idforwarded viaget_config.pick_variant_for_qualitywith2K → 1440/4K → 2160mapping,filter_audio_onlypreserving HLS, handles public / private-hash / showcase URL shapes.&unescaping and deterministic sort), Flickrphotosets.getPhotoswith URL-encodedapi_key. Generic<img>scraper is a separateextract_genericexport so the plugin does not claim ownership of every HTTP page. Scheme guard blocksdata:,javascript:,mailto:,blob:from relative resolution. Post-processing: dedupe → min-resolution filter (preserves unknown dimensions) → auto-name with album-id sanitisation.Hardening and review
PluginErrorenum viathiserrorwithSerdeJson(#[from])for source-chain preservation..unwrap()outside tests, no#[allow(dead_code)], no#[allow(unused)].unsafe { host_fn(...) }call has a documented SAFETY invariant list (symbol registration, ABI, capability gating, owned-JSON I/O) inline — no cross-plugin references.manual_unwrap_or_defaulton wasm32-wasip1 target (all 3 plugins)-largevariantpick_variant_for_qualityon unsorted inputapi_key/album_idnow URL-encodedwindow.playerConfigwith fallback.unwrap()with.is_some_and(...)in the scheme guardTests
cargo test --target x86_64-unknown-linux-gnuon each crate.wasm32-wasip1andx86_64-unknown-linux-gnu) for all 3 crates.opt-level = \"z\"+ strip.Release WASM sizes
Test plan
Summary by cubic
Adds three new WASM media plugins—
vortex-mod-soundcloud,vortex-mod-vimeo, andvortex-mod-gallery—to extract tracks, videos, and image galleries via the hosthttp_request. Implements Linear task 25; latest update strengthens Vimeo player-config scanning with a single-pass parser that tracks JS strings and only accepts real assignments before the first brace outside strings.New Features
vortex-mod-soundcloud:/resolvev2 for tracks and playlists/likes; artwork upgrade to 500×500; readsclient_idviaget_config.vortex-mod-vimeo: oEmbed + player-config JSON for MP4/HLS; supports public and private-hash URLs; quality picker and optional audio-only.vortex-mod-gallery: Imgur albums, Reddit galleries/single-image, Flickr photosets; generic<img>scraping; dedupe, min-resolution filter, auto filenames.Bug Fixes
-large; newUnsupportedResourceKind { kind, url }for kind-mismatch inensure_*; clearer message for artist URLs (not supported yet).{, accepts only plain=(rejects==,===,!=,!==,<=,>=,=>), ignores=inside JS strings, and now uses a single-pass scanner that tracks JS string state and finds the first{outside strings; parser accepts single-quoted JS object literals, preserves non-ASCII; deterministic CDN;default_qualityhonored; showcase rejected; private-hash source URL preserved.url_o/url_land requests all neededextras; Reddit native gallery preserves item order viagallery_data.items; retains earlier fixes (query/fragment stripping, better URL resolution,.avifaccepted, tighter regexes, case-insensitive HTTP scheme).validate_and_split..expect(...)with messages across SoundCloud, Vimeo, and Gallery, removing remainingunwrap()s.Written for commit 26b3159. Summary will update on new commits.
Summary by CodeRabbit