Skip to content

refactor(server): extract the enrichment route handlers into routers/enrichment.py (R3) - #868

Merged
byrongamatos merged 1 commit into
mainfrom
refactor/r3-router-enrichment-routes
Jul 11, 2026
Merged

byrongamatos merged 1 commit into
mainfrom
refactor/r3-router-enrichment-routes

Conversation

@byrongamatos

@byrongamatos byrongamatos commented Jul 11, 2026

Copy link
Copy Markdown
Contributor

The 14 /api/enrichment/* routes — status, kick/cancel, per-song state, the Match-Review queue (accept/reject/pick/search/rematch/refresh/states), and the AcoustID fingerprint identify endpoints — plus the route-exclusive candidate sanitizer → lib/routers/enrichment.py.

Bodies verbatim except @app@router and the seam reads (meta_dbappstate.meta_db, CONFIG_DIRappstate.config_dir). The enrichment engine (transport, matcher, worker, upload caps) already lives in lib/enrichment.py from the earlier subsystem move and is reached as enrichment.X. No new seams.

server.py: 2,925 → 2,638 (−287).

Verification

  • pyflakes clean; route set identical (143).
  • Full pytest 2396 passed (the enrichment route + Match-Review + identify cases, which fake the network on the enrichment module). npm run lint 0; Codex 0 findings.

🤖 Generated with Claude Code

Summary by CodeRabbit

  • New Features
    • Added enrichment pipeline status and controls.
    • Added match-review queue actions, including accept, reject, and candidate selection.
    • Added MusicBrainz search and AcoustID-based song identification through uploads or library audio.
  • Refactor
    • Reorganized enrichment API handling while preserving existing endpoint behavior.

…enrichment.py (R3)

The 14 /api/enrichment/* routes — status, kick/cancel, per-song state, the
Match-Review queue (accept/reject/pick/search/rematch/refresh/states), and the
AcoustID fingerprint identify endpoints — plus the route-exclusive candidate
sanitizer move to lib/routers/enrichment.py. Bodies verbatim except @app->@router
and the seam reads (meta_db->appstate.meta_db, CONFIG_DIR->appstate.config_dir).
The enrichment engine (transport, matcher, worker, upload caps) already lives in
lib/enrichment.py from the earlier subsystem move and is reached as enrichment.X.
No new seams.

server.py: 2,925 -> 2,638 (-287).

Verified: pyflakes clean; route set identical (143); full pytest 2396 passed (the
enrichment route + Match-Review + identify cases, which fake the network on the
enrichment module). eslint 0.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@coderabbitai

coderabbitai Bot commented Jul 11, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

📝 Walkthrough

Walkthrough

The enrichment API surface was moved from server.py into a dedicated FastAPI router. The router preserves status, control, review, search, and fingerprint endpoints, while server.py registers it through app.include_router.

Changes

Enrichment router extraction

Layer / File(s) Summary
Enrichment status, control, and review endpoints
lib/routers/enrichment.py
Adds enrichment status, song lookup, pass control, progress polling, refresh, review queue, review actions, and candidate sanitization endpoints.
Search and fingerprint integrations
lib/routers/enrichment.py
Adds MusicBrainz search proxying and AcoustID identification for uploaded audio and existing library songs with validation and transport handling.
Server registration and extraction documentation
server.py, CHANGELOG.md, docs/size-exemptions.md
Removes inline enrichment handlers, registers the router, removes unused imports, and updates extraction documentation.

Estimated code review effort: 4 (Complex) | ~45 minutes

Possibly related PRs

  • got-feedBack/feedBack#864: Updates multipart Content-Length handling for the enrichment identify endpoint later moved into the router.
🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly describes the main change: extracting server enrichment route handlers into routers/enrichment.py.
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 refactor/r3-router-enrichment-routes

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

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick comments (2)
lib/routers/enrichment.py (1)

86-86: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Ruff B008 here is a false positive for FastAPI. Body(...) (and Query/Depends) in parameter defaults is the intended FastAPI idiom, so don't rewrite these handlers (also flagged at Lines 114, 163, 210). To silence it repo-wide across all routers, add the FastAPI factories to Bugbear's immutable-call allowlist in your Ruff config, e.g.:

[tool.ruff.lint.flake8-bugbear]
extend-immutable-calls = ["fastapi.Body", "fastapi.Query", "fastapi.Depends", "fastapi.File", "fastapi.Form"]
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@lib/routers/enrichment.py` at line 86, Add the FastAPI dependency factories
Body, Query, Depends, File, and Form to Ruff’s flake8-bugbear
extend-immutable-calls configuration, preserving the existing handler signatures
such as api_enrichment_rematch and the other router handlers.

Source: Linters/SAST tools

server.py (1)

1807-1830: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Trailing blank-line artifact from the deleted handler block. ~24 consecutive blank lines remain where the inline /api/enrichment/* handlers used to live. Collapse to the normal 2-blank-line spacing before the next top-level statement to keep the diff clean (and avoid an E303-style lint if Ruff runs over server.py).

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@server.py` around lines 1807 - 1830, Remove the excessive consecutive blank
lines left by the deleted inline enrichment handlers in server.py, restoring
standard two-blank-line spacing before the next top-level statement. Do not
alter the surrounding handler or statement logic.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Nitpick comments:
In `@lib/routers/enrichment.py`:
- Line 86: Add the FastAPI dependency factories Body, Query, Depends, File, and
Form to Ruff’s flake8-bugbear extend-immutable-calls configuration, preserving
the existing handler signatures such as api_enrichment_rematch and the other
router handlers.

In `@server.py`:
- Around line 1807-1830: Remove the excessive consecutive blank lines left by
the deleted inline enrichment handlers in server.py, restoring standard
two-blank-line spacing before the next top-level statement. Do not alter the
surrounding handler or statement logic.

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: 3c94e406-61be-4836-8775-6ef6a0daed42

📥 Commits

Reviewing files that changed from the base of the PR and between f00ba22 and 45a646b.

📒 Files selected for processing (4)
  • CHANGELOG.md
  • docs/size-exemptions.md
  • lib/routers/enrichment.py
  • server.py

@byrongamatos
byrongamatos merged commit b7624b7 into main Jul 11, 2026
5 of 6 checks passed
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