perf(paths): resolve the library root once, not on every path check (55x fewer stats)

[byrongamatos]

Found while profiling Byron's 2-fps report. Separate from the renderer bug (song-preview#7) — this one is the server side.

The bug

Path.resolve() is a filesystem call: it lstats every component of the path. Both _resolve_dlc_path and safe_join re-resolved their root on every call — and those run once per song, per art fetch, per scanned row.

On a real 50,944-song library, strace on the running server caught it issuing ~23,500 stat/lstat calls per second, re-walking the same three parent directories forever:

  11,599  /mnt/big/feedpak      <- the same dir, 11.6k times in 5 seconds
   5,799  /mnt/big
   5,799  /mnt

That 2:1:1 ratio is the parent-walk signature of resolve(). It burned ~50% of a core.

It is worst exactly where big libraries live: this one is on an NTFS-3G (FUSE) mount, so every stat is a userspace round trip through mount.ntfs-3g (which was itself visible burning CPU in top). The cost was the constant **re-**resolution, not the work.

The fix

A root is fixed for the life of the process. Resolve it once — safepath.resolved_root, an lru_cached helper shared by both call sites.

Measured, 5,000 lookups against a real library path:

	stat syscalls	time
before	15,264	54.2 ms
after	277	0.8 ms

55× fewer.

Containment is unchanged — the part that matters

• safe_join still resolves the CANDIDATE on every call. Following the candidate's symlinks is the zip-slip / traversal defence, so it is never cached. Only the server-owned root is.
• _resolve_dlc_path keeps its lexical containment check (it deliberately does not follow symlinks, so junction-mounted libraries keep working).

Re-pinned in tests: .. traversal, backslash traversal, POSIX-absolute, Windows drive-absolute, embedded NUL, empty name — and a symlink escaping the root is still refused, which is the one that would catch a bad cache.

Documented tradeoff

If a root's symlink/junction is re-pointed at a new target while the server is running, the old target stays in effect until restart. That is fine for a library path fixed at startup, and the cache is keyed on the Path, so switching library dir is a different key. Called out in the docstring rather than left implicit.

Tests

New tests/test_resolved_root_cache.py: root resolved once across 500 lookups (the regression), a different root is a different cache entry, plus the full containment contract. Full suite green: 2,597 passed, 4 skipped.

Summary by CodeRabbit

• Bug Fixes

  • Improved handling of DLC library paths for more consistent repeated lookups.
  • Preserved protections against directory traversal and symlink-based escapes.
  • Continued resolving requested paths to ensure paths outside approved locations are rejected.

• Tests

  • Added coverage for repeated path lookups, multiple library roots, valid paths, and escape attempts.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: 3bca14ca-9e5d-43a0-bd99-4fee2907d8e5

📥 Commits

Reviewing files that changed from the base of the PR and between 2991612 and 5c606e0.

📒 Files selected for processing (3)

• lib/dlc_paths.py
• lib/safepath.py
• tests/test_resolved_root_cache.py

---

📝 Walkthrough

Walkthrough

Adds bounded caching for canonical server-owned roots in safe_join and DLC path resolution, while continuing to resolve candidate paths per call and enforce containment. New tests verify cache behavior, root partitioning, traversal protection, symlink escapes, and valid paths.

Changes

Cached path resolution

Layer / File(s)	Summary
Cached root resolution integration lib/safepath.py, lib/dlc_paths.py	Adds cached canonical root resolution and uses it in safe_join and _resolve_dlc_path while preserving containment checks.
Cache and containment validation tests/test_resolved_root_cache.py	Verifies cache hits and partitioning, escape rejection, symlink protection, and successful in-root resolution.

Estimated code review effort: 3 (Moderate) | ~20 minutes

Possibly related PRs

• got-feedBack/feedBack#766: Also modifies safe_join input and containment handling.
• got-feedBack/feedBack#843: Also modifies DLC path containment logic in _resolve_dlc_path.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Description check	⚠️ Warning	The description is relevant, but it does not follow the required template and omits the issue link, checklist, and feedpak-surface section.	Rewrite the PR description using the repository template: add a What section with the issue link, include or remove feedpak surface as appropriate, and fill the checklist.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title clearly summarizes the main change: caching library-root resolution to reduce path checks and stat calls.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

✨ Finishing Touches

📝 Generate docstrings

• Create stacked PR
• Commit on current branch

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch perf/resolve-library-root-once

---

_{Comment @coderabbitai help to get the list of available commands.}