Skip to content

chore(alpha): combined housekeeping — console.debug + tooltip a11y + ffprobe noise filter (closes #847, #950, #951; supersedes #952 #953)#954

Open
Salem874 wants to merge 5 commits into
alphafrom
chore/combined-alpha-housekeeping-2026-06-19
Open

chore(alpha): combined housekeeping — console.debug + tooltip a11y + ffprobe noise filter (closes #847, #950, #951; supersedes #952 #953)#954
Salem874 wants to merge 5 commits into
alphafrom
chore/combined-alpha-housekeeping-2026-06-19

Conversation

@Salem874

Copy link
Copy Markdown
Contributor

Summary

Combined housekeeping wave for the alpha channel. Bundles the three commits from PRs #952 + #953 that actually apply to alpha's current state, dropping the two that are already a no-op on alpha. Single merge so reviewers can see the full housekeeping wave atomically without PR stacking.

Per-commit applicability audit against actual alpha codebase

I verified each commit from #952 + #953 by grepping the actual alpha codebase (not just commit logs) before deciding what to cherry-pick. Two commits were dropped because alpha already has the fixes:

Originally in Commit Alpha state Decision
#952 `21c226fe` milestone numbering across 6 files Already correct on alpha (M9-1..M9-6 set during M9 Spotify work; the bug only ever existed on main) SKIPPED
#952 `99b5d0fc` GAMDL v3.7.2-v3.7.3 + v3.7.4 audit doc backfill Both audit docs already exist on alpha (came in via #928 brand refresh) SKIPPED
#952 `205f1526` console.log → console.debug in App.tsx deep-link handler Alpha still has `console.log` at App.tsx:951 APPLIED
#952 `ea9b133f` Cancel/Retry tooltip title=aria-label a11y Alpha has the same title="Cancel"/title="Retry" + aria-label="Cancel download"/"Retry download" mismatch APPLIED
#953 `2d7701e0` ffprobe demuxing-noise filter (#847) Alpha has `is_python_traceback_noise` but no ffprobe sibling APPLIED

All three cherry-picks landed clean — no conflict resolution required (git auto-merged `download_queue.rs` cleanly even though #911 / Spotify work moved adjacent regions around).

What's in this PR

console.log → console.debug (closes #951)

src/App.tsx:951 deep-link handler used `console.log()`. The prevailing convention elsewhere is `console.debug` (`useTheme.ts:188-196`, `main.tsx:90`) so dev-only diagnostics are suppressed at the browser's default log level. The line above already shows a user-facing toast, so the console line is purely dev-diagnostic.

QueueItem Cancel/Retry tooltip a11y (closes #950)

QueueItem.tsx had `title="Cancel"` / `title="Retry"` but `aria-label="Cancel download" / "Retry download"` — sighted-hover tooltip didn't match screen-reader announcement. Sibling to the #945 a11y wave shipped in main via PR #947 (Retry-without-Wrapper, Open File, Open Folder buttons). Updated `title` to the longer form so both match.

ffprobe demuxing-noise filter (closes #847)

ffprobe's "Invalid data found when processing input" warning fires from stderr on virtually every track during enrichment — modern ffmpeg's partial-moov-atom warning at byte 0. Downloads complete; ffprobe falls through to a valid result on retry. But ~20 noise lines per album turned the activity log into static.

Modelled exactly on the shipped #660 pattern (`is_python_traceback_noise` + non-verbose suppression in `download_queue.rs::emit_subprocess_line` gate). Verbose mode still surfaces everything; on-disk activity-log writer still records every line.

6 unit tests pin the contract — all green on alpha (1412 other tests filtered-but-present, confirming alpha's much-richer state vs main).

Why combined into one PR

User asked for the housekeeping work bundled into a single alpha-targeting merge to avoid PR stacking + race conditions. Three small commits, three closing issues, one merge.

Sibling PRs (to close as superseded)

Note: `#948` (release-please's `chore(main): release 1.10.1`) intentionally excluded — it's auto-generated for the main stable cut and bumps versions across package.json / Cargo.toml / tauri.conf.json to `1.10.1`. Alpha is on `1.11.0-alpha.25` and would need different version bumps; release-please regenerates the PR from main's commit history, so it can't sensibly retarget to alpha.

Verification

  • ✅ `cargo check` clean
  • ✅ `cargo test --lib is_ffprobe_demux_noise` → 6/6 pass
  • ✅ All 3 cherry-picks auto-merged with no manual conflict resolution
  • ✅ Per-commit applicability verified by grepping actual alpha codebase before applying

🤖 Generated with Claude Code

Salem874 added 3 commits June 19, 2026 08:01
…dler (#951)

The deep-link handler logged every received URL via `console.log()`. Other dev-only diagnostics in the codebase (`src/hooks/useTheme.ts:188-196`, `src/main.tsx:90`) use `console.debug` so the lines stay quiet at default browser log levels but remain visible when verbose mode is enabled.

The line above (945-948) already shows a user-facing toast announcing the deep-link event, so the console line is purely dev-diagnostic and belongs on `console.debug`.

One-line edit + inline comment pointing to the convention.

Closes #951
PR #945 (merged in #947) aligned `title` with `aria-label` for three QueueItem buttons (Retry-without-Wrapper, Open File, Open Folder) so the sighted hover-tooltip and the screen-reader announcement say the same thing. Two sibling buttons in the same file were missed:

- Cancel button (line 617) had `title="Cancel"` but `aria-label="Cancel download"`.
- Retry button (line 633) had `title="Retry"` but `aria-label="Retry download"`.

Sighted-mouse users hover and see "Cancel"; screen-reader users hear "Cancel download". The longer form is the right one — it's specific (cancels THIS row, not e.g. the whole queue) and matches the verbose pattern the rest of the file establishes.

Updates `title` on both buttons to match the existing `aria-label`. Two-line edit. No behavioural change.

Closes #950
…rbose activity log (#847)

`Error during demuxing: Invalid data found when processing input` fires from ffprobe stderr on virtually every track during enrichment — modern ffmpeg's partial-moov-atom warning at byte 0. Downloads complete; ffprobe falls through to a valid result on retry. But the noise produces ~20 entries per album in the activity log, reported in #847 as the most-aggravating recurring noise.

Implementation modelled exactly on the shipped #660 pattern (`is_python_traceback_noise` + non-verbose suppression in `download_queue.rs::emit_subprocess_line` gate):

- New `process::is_ffprobe_demux_noise(line: &str) -> bool` in `utils/process.rs`. Match keys on the structural `[in#<digit>/<demuxer-list> @ 0x<hex>]` prefix plus both required tail substrings (`Error during demuxing:` AND `Invalid data found when processing input`). Tight enough that genuine ffmpeg demuxer errors that share words pass through; loose enough to survive future demuxer-list shifts (the comma-separated demuxer names vary by ffmpeg version).
- Wired into both stdout (line 10958) and stderr (line 11155) reader gates in `download_queue.rs` — `is_known_noise` now combines both `is_python_traceback_noise` and `is_ffprobe_demux_noise`. The 5th arg to `emit_subprocess_line` (`show_in_ui`) stays `verbose || !is_known_noise` so the on-disk activity-log writer still records every line — forensic record intact for support requests.

6 new unit tests pin the contract:
- Canonical Apple Music shape (verbatim from the #847 issue report)
- Leading-whitespace + wider-hex-pointer tolerance
- Alternate demuxer lists (aac, wav,mp3) — future-proof against ffmpeg auto-selection changes
- Genuine ffmpeg errors pass through (muxer errors, parameter errors, generic "Invalid data" without the demuxing prefix)
- Empty / unrelated / traceback lines pass through
- Wrapped GAMDL `[INFO     HH:MM:SS]` prefix doesn't false-positive
- Requires `0x<hex>` pointer form — gate fails open (noise resumes) if upstream switches format rather than over-suppressing

Closes #847
@Salem874

Copy link
Copy Markdown
Contributor Author

PR #948 cannot be combined — release-please version mismatch

User asked to combine PR #948 into this PR. Verified by attempting the cherry-pick:

$ git cherry-pick 0a702277  # #948's release-please 1.10.1 bump commit
Auto-merging src-tauri/tauri.conf.json
CONFLICT (content): Merge conflict in src-tauri/tauri.conf.json

PR #948 contains only 2 commits (chore(main): release 1.10.1 + chore(release): update Cargo.lock) which bump versions in:

  • README.md (1.10.0 → 1.10.1)
  • Project_Plan.md (1.10.0 → 1.10.1)
  • package.json / package-lock.json
  • src-tauri/Cargo.toml / Cargo.lock
  • src-tauri/tauri.conf.json
  • .release-please-manifest.json

Alpha is on 1.11.0-alpha.25. Cherry-picking #948 onto alpha either conflicts (current outcome) or silently downgrades alpha to 1.10.1 (catastrophic — breaks the alpha-release counter sequence).

PR #948 is release-please's PR — it auto-regenerates from main's conventional-commit history. It can only live on main. Treating it as superseded by this PR isn't possible; it stays in its own lifecycle. After this PR merges to alpha and alpha → main → realign-alpha completes, release-please will regenerate #948 with the right base.

This PR (#954) therefore stands alone as the alpha housekeeping wave. CI workflows are being manually dispatched (PRs targeting alpha don't auto-trigger ci.yml/codeql/licences — only pr-security which already passed).

Salem874 added 2 commits June 19, 2026 08:39
…Rust deps

Licence-compliance CI on this PR flagged 6 direct Rust deps that exist on alpha (from M9 Spotify integration #919, Profile Bundle #899, and the meedya-fingerprint/meedya-lyrics shared crates landing) but were never added to ACKNOWLEDGEMENTS.md.

Backfilled in alphabetical order:

| Crate | Where it shows up |
|---|---|
| aes-gcm 0.10 | Profile Bundle export passphrase encryption + credential vault |
| meedya-fingerprint (git) | Shared audio-fingerprint primitives from MeedyaSuite-core |
| meedya-lyrics (git) | Shared lyrics primitives from MeedyaSuite-core (TTML / Lyricsfile / classifier / LRC offset round-trip) |
| pbkdf2 0.12 | Password-based key derivation (paired with aes-gcm) |
| rand 0.8 | Salt + nonce derivation, retry jitter |
| rusqlite 0.31 | Library Index database + Profile Bundle manifest persistence |

Verified by running both `npm run check:acknowledgements` (set-coverage) and `npm run check:upstream-licences` (drift check against actual upstream licence strings) — both now pass with 49 Rust + 26 npm direct deps covered and zero advisories.

Required by alpha's licence-compliance CI before this PR can merge.
CI on this PR surfaced two pre-existing alpha-state issues blocking merge — neither caused by the #954 cherry-picks. Both fixed in-line so the housekeeping wave can land cleanly.

## undici 7.0.0-7.27.2 HIGH severity vulnerability

Frontend Security Audit (macOS + Ubuntu + Windows) failed with:

```
undici  7.0.0 - 7.27.2
Severity: high
undici vulnerable to TLS certificate validation bypass via dropped requestTls in SOCKS5 ProxyAgent
undici vulnerable to cross-user information disclosure via shared cache whitespace bypass
```

Same root cause as the #943 fix that shipped to main via PR #947 — alpha hadn't received the override yet. Mirrored the same `"undici": "^7.28.0"` line into alpha's `overrides` block. Verified: `npm audit --audit-level=high` returns `found 0 vulnerabilities`.

## services::download_index::queries::tests::search_activity_events_filters_by_level_and_message — macOS-only flake

Backend (macos-14) failed with:

```
called `Result::unwrap()` on an `Err` value: SqliteFailure(Error { code: ReadOnly, extended_code: 1032 }, Some("attempt to write a readonly database"))
```

Root cause: `temp_db()` builds a unique-per-call directory name from `SystemTime::now().as_nanos()`. macOS clock resolution is coarser than Linux's, so two tests in the same process can call `temp_db()` in the same nanosecond → both land on the same DB path → one opens for read+write, the other tries to write and gets SQLite's read-only error.

Fix: replace nanosecond-based uniqueness with `uuid::Uuid::new_v4().simple()` which guarantees per-call uniqueness regardless of clock resolution. The `uuid` crate is already a direct dep (per ACKNOWLEDGEMENTS.md) so no new dep added. Inline comment explains the failure mode + the original CI run for future archaeology.

Verified: `cargo test --lib search_activity_events_filters_by_level` → ok. Whole-suite test count unchanged (1418 tests total, 1 was the flake).

Neither fix is alpha-specific or PR-specific — both should remain on alpha going forward.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant