fix(media): accept portrait video resolutions - #6058
Conversation
jedwards27
left a comment
There was a problem hiding this comment.
A Team review — CLEAR at ad653f6dddb3da27f09c78bbc0a2ccb27322f3ed
Carl, Mongo, and Princess Donut independently reviewed this exact head and found no material correctness, security, compatibility, reliability, or user-trust issues.
crates/buzz-media/src/validation.rs:373-381correctly normalizes encoded dimensions to short/long edges, preserving the existing 2160×3840 resource ceiling while removing orientation bias.- Validation remains before S3 publication (
crates/buzz-media/src/upload.rs:417-425,465-467). - The revised 422 text matches the orientation-independent contract (
crates/buzz-media/src/error.rs:75-79).
Exact-head evidence across the three independent passes:
just cipassed, including Rust workspace tests (2440 passed, 15 ignored), Desktop suites, and Mobile (1465 passed).cargo test -p buzz-mediapassed (120 passed; the existing live-MinIO test remained ignored).cargo clippy -p buzz-media --all-targets -- -D warnings, formatting, and diff checks passed.- The formerly blocked H.264 recording (SHA-256
9f6cb239352a204ea706155462a84d3c228c78b3d53fb912418f1942223c6621, 1206×2622) validated and returned those dimensions. - Causal mutations were effective: restoring the old orientation-sensitive predicate rejected the portrait artifact/test, while removing either short-edge or long-edge guard failed its matching boundary test.
Residual evidence gap: we did not repeat the live authenticated MinIO upload round-trip. Validation ordering was traced directly, the complete repository gate passed, and the exact production-shaped artifact plus causal controls exercised the changed boundary, so this is not material for this predicate-only change.
Merge recommendation: approve this exact head. Any head movement reopens review.
themiguelamador
left a comment
There was a problem hiding this comment.
Reviewed the exact head ad653f6dddb3da27f09c78bbc0a2ccb27322f3ed exhaustively. No findings.
The short/long-edge normalization preserves the existing 2160×3840 pixel envelope while removing the orientation bias, the parser reports encoded H.264 sample-entry dimensions (so the comparison targets the intended resource bound), and the error contract and boundary coverage are consistent.
Verification: cargo test -p buzz-media (120 passed; one existing live-service integration test ignored), strict all-target clippy, workspace formatting, and diff checks all passed.
wesbillman
left a comment
There was a problem hiding this comment.
Carl, an automated reviewer, commenting via Wes’s GitHub account.
Royal Court consolidated review: CLEAR
Reviewed exact head ad653f6dddb3da27f09c78bbc0a2ccb27322f3ed. Carl, Princess Donut, Mongo, and Mordecai found no material correctness, security, compatibility, reliability, or user-trust defects.
crates/buzz-media/src/validation.rs:373-381normalizes the parser-reported encoded dimensions withmin/max. This removes orientation bias while preserving both existing resource ceilings: short edge ≤ 2160 and long edge ≤ 3840.- The boundary tests cover the exact accepted portrait limit and independent +1 failures for each guard (
validation.rs:2554-2589); the existing oversized landscape test remains. - Returned and persisted metadata preserves the original width/height order (
validation.rs:384-389,upload.rs:469-478), so portrait descriptors are not accidentally transposed. - Full validation still precedes S3 publication (
upload.rs:417-466), so the broader orientation acceptance does not create a storage bypass. mp4 0.14.0returns these dimensions asu16, making the conversions and comparisons overflow-safe.- The revised 422 text accurately states the orientation-independent contract (
error.rs:75-79). git diff --checkpassed. A repository-tree search found no second active 3840×2160 validator or stale user-facing resolution contract. GitHub reports the exact head mergeable; a merge-tree check against currentorigin/mainfound no conflicts.
The head has not moved since the earlier exact-head validation recorded on this PR, which included the complete repository gate, cargo test -p buzz-media, strict clippy, the real 1206×2622 H.264 artifact, and causal mutation checks. We did not duplicate those CI-equivalent suites.
Non-blocking caveat: the branch is 41 commits behind current main. It currently merges cleanly, but any rebase or other head movement reopens this verdict.
wesbillman
left a comment
There was a problem hiding this comment.
Carl, an automated reviewer, approving via Wes’s GitHub account.
Royal Court approval for exact head ad653f6dddb3da27f09c78bbc0a2ccb27322f3ed. No material findings. Any head movement reopens review.
Signed-off-by: Carl <5f365698229751c0461f57bb03a4e93134e6e936bd7039ebe7b737282a43c754@buzz.block.builderlab.xyz>
b0b7068 to
a3ce15a
Compare
…el-directory * origin/main: perf(desktop): move five hot renderer paths from JS into Rust (#6024) fix(media): accept portrait video resolutions (#6058) refactor(desktop): coordinate TTS playback fix(desktop): hide archived channels from #/Tab autocomplete (#6156) Unify mobile channel details (#6113) Signed-off-by: Hayt <9e1c23a3fd83f61da34420e4e88ff1b16e45cafcc0cd9019eb07d4ecfa8ca9b0@buzz.block.builderlab.xyz>
…c-agent-commit-identity * origin/main: fix(buzz-acp): loosen workspace-scan guardrail to allow named paths (#6261) fix(buzz-dev-mcp): expand leading ~ in read_file/str_replace paths (#6271) perf(desktop): move five hot renderer paths from JS into Rust (#6024) fix(media): accept portrait video resolutions (#6058) fix(desktop): hide archived channels from #/Tab autocomplete (#6156) Unify mobile channel details (#6113) Signed-off-by: Duncan <dcfd242e557282d7a1e2cf2e6877522682f1e5c6156dc92ca7d90eaedd3b0f95@buzz.block.builderlab.xyz>
…urneys * origin/main: chore: serialize mobile pre-push checks (block#6322) fix(buzz-acp): loosen workspace-scan guardrail to allow named paths (block#6261) fix(buzz-dev-mcp): expand leading ~ in read_file/str_replace paths (block#6271) perf(desktop): move five hot renderer paths from JS into Rust (block#6024) fix(media): accept portrait video resolutions (block#6058) fix(desktop): hide archived channels from #/Tab autocomplete (block#6156) Unify mobile channel details (block#6113) Revert "fix(acp): gate relay-signed workflow messages on their attributed author" (block#6311) fix(desktop): morph the drawer panel icon instead of sliding it (block#6306) feat(desktop): refine repository-aware project workspaces (block#6003) Fix mobile Activity thread navigation (block#5850) Signed-off-by: Jude Edwards <judeedwards@squareup.com>
…oundation * origin/main: Add appearance preference previews (#6193) fix(desktop): restore emoji recents (#6263) chore: serialize mobile pre-push checks (#6322) fix(buzz-acp): loosen workspace-scan guardrail to allow named paths (#6261) fix(buzz-dev-mcp): expand leading ~ in read_file/str_replace paths (#6271) perf(desktop): move five hot renderer paths from JS into Rust (#6024) fix(media): accept portrait video resolutions (#6058) Co-authored-by: Taylor Ho <taylorkmho@gmail.com> Signed-off-by: Taylor Ho <taylorkmho@gmail.com>
…-shortcut * origin/main: (341 commits) feat(desktop): make the Projects overview follow the selected section (block#6335) refactor(desktop): coordinate TTS playback (block#6341) fix(desktop): show complete repository trees (block#5102) Add appearance preference previews (block#6193) fix(desktop): restore emoji recents (block#6263) chore: serialize mobile pre-push checks (block#6322) fix(buzz-acp): loosen workspace-scan guardrail to allow named paths (block#6261) fix(buzz-dev-mcp): expand leading ~ in read_file/str_replace paths (block#6271) perf(desktop): move five hot renderer paths from JS into Rust (block#6024) fix(media): accept portrait video resolutions (block#6058) fix(desktop): hide archived channels from #/Tab autocomplete (block#6156) Unify mobile channel details (block#6113) Revert "fix(acp): gate relay-signed workflow messages on their attributed author" (block#6311) fix(desktop): morph the drawer panel icon instead of sliding it (block#6306) feat(desktop): refine repository-aware project workspaces (block#6003) Fix mobile Activity thread navigation (block#5850) perf(desktop): parallelize relay agent directory rebuild (block#6258) Refine the mobile emoji picker (block#5853) fix(desktop): exclude archived agents from nest, order regeneration (block#5905) Add font size and conversation density preferences (block#5644) ... Signed-off-by: John Funge <funge@squareup.com> # Conflicts: # desktop/src/features/messages/ui/MessageComposer.tsx # desktop/src/features/settings/ui/KeyboardShortcutsCard.tsx
…ress * origin/main: fix(desktop): distinguish duplicate agent devices (#6337) feat(desktop): close Buzz window with Cmd+W (#6314) refactor(prompt): simplify Buzz agent guidance (#6340) feat(desktop): make Projects workspaces selectable (#6368) Add Buzz-native collaboration benchmarks (#6264) Polish mobile timeline and emoji interactions (#6297) feat(desktop): make the Projects overview follow the selected section (#6335) refactor(desktop): coordinate TTS playback (#6341) fix(desktop): show complete repository trees (#5102) Add appearance preference previews (#6193) fix(desktop): restore emoji recents (#6263) chore: serialize mobile pre-push checks (#6322) fix(buzz-acp): loosen workspace-scan guardrail to allow named paths (#6261) fix(buzz-dev-mcp): expand leading ~ in read_file/str_replace paths (#6271) perf(desktop): move five hot renderer paths from JS into Rust (#6024) fix(media): accept portrait video resolutions (#6058) fix(desktop): hide archived channels from #/Tab autocomplete (#6156) Unify mobile channel details (#6113) Revert "fix(acp): gate relay-signed workflow messages on their attributed author" (#6311) Signed-off-by: Taylor Ho <taylorkmho@gmail.com> # Conflicts: # desktop/src/features/messages/ui/MentionAutocomplete.test.mjs
Summary
Why
The iOS simulator produces portrait H.264 recordings such as 1206×2622. The relay previously checked
width <= 3840 && height <= 2160, so the equivalent portrait dimensions were rejected while landscape dimensions were accepted.Validation
bin/just cicargo test -p buzz-media(120 passed; one MinIO integration test ignored by its existing live-service guard)cargo clippy -p buzz-media --all-targets -- -D warnings/Users/judeedwards/.buzz/.scratch/pr5874-mongo-evidence/native/head-20260816T050317Z/video-bitexact.mp4validated throughvalidate_video_file: 1206×2622 accepted