Repository navigation
refactor(bot): move title comparison out of utils into services - #2355
Conversation
TitleComparisonService holds `private readonly cache: Map<string, ArtistTitle>`, so it is a stateful service, not a util. Last of the five classes #1991 enumerated. Eleven references across three shapes, all rewritten: two from queueDisplay, eight from trackUtils, and one jest.mock() literal in the tests/ tree, which is the second source tree a src-only scan misses. The eight from trackUtils collapse back to '../titleComparison'. They were relative siblings before #1968 split the tree, became '../../../utils/music/titleComparison' when trackUtils moved to services in the slice before this one, and are siblings again now that both live under services/musicManagement. One outward dependency needed the reverse rewrite, since the moved code now sits one level deeper relative to utils: '../../misc/stringUtils' becomes '../../../utils/misc/stringUtils', in service.ts and in the jest.mock() literal in service.spec.ts. Closes #1991 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Pkt4D9yyHTrVhsvDYvw5an
|
Failed to generate code suggestions for PR |
![graphify-labs[bot]](https://avatars.githubusercontent.com/in/4356270?s=60&v=4){kind=small}
There was a problem hiding this comment.
Graphify reviewed this change.
Looks safe to merge — no coupling regressions and no blocking issues, checked against the code graph (not a self-assessment).
Graphify review — findings
Relocates the titleComparison module from utils/music to services/musicManagement, updating all importers (queueDisplay, trackProcessor, and both test suites) to the new path. Pure move/import-path refactor with no behavioural change.
No blocking issues surfaced. 1 lower-confidence candidate did not survive cross-model review.
Analysis details — impact, health, verification
Impact & health
Graphify review
Impact — 93 functions depend on the 68 functions this change touches.
Health — this change adds coupling hotspots:
- new:
handleEvents()— 5 callers, 15 callees - new:
createQueueEmbed()— 4 callers, 6 callees - new:
routeButtonAction()— 1 callers, 10 callees - new:
runInteraction()— 1 callers, 7 callees
Verification — 93 functions in the blast radius were not formally verified this run (proofs are advisory here).
Gate & verification
graphify gate
PASS — objectively clean (no health regressions, tests not run — proofs not run this pass (advisory)). Grounded, not self-assessed.
Advisory (not blocking):
- verification_scope: 81 function(s) in the blast radius were not formally verified this run
· 4 more finding(s) on lines outside this diff (see the check run).
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
No issues found across 10 files
Auto-approved: Pure refactor moving the stateful TitleComparisonService from utils to services; diff shows only import path updates and file renames, no behavior change.
Re-trigger cubic
SonarCloud failed this PR on new_coverage 75.6 percent against an 80 percent threshold. Same cause as the sibling slice: a pure move adds no logic, but Sonar counts moved files as new code, so index.ts sitting at 0 percent since long before the move became gate-blocking here. Tested against the real TitleComparisonService rather than a double, so the assertions cover the facade wiring through to working behaviour, the per-instance cache, and the custom threshold, instead of only proving a mock was called. Two things the writing turned up. The shared utils barrel cannot be requireActual'd from a spec: it re-exports database/prismaClient, which is ESM and fails under the CommonJS sandbox with "Must use import to load ES Module". Only the three functions the service uses are mocked. stringUtils reaches similarity through its own deep specifier and stays real. extractArtistTitle does not split "artist - title" input. Both artistTitlePatterns and artistPatterns ship as empty arrays, so applyPatterns matches nothing and every such string takes the fallback branch as artist "Unknown". The test asserts that real behaviour rather than the intended behaviour; the gap is filed separately. Coverage on the moved set goes from 72.46 to 100 percent of lines. No threshold, exclusion or gate configuration was changed.
There was a problem hiding this comment.
Graphify reviewed this change.
Looks safe to merge — no coupling regressions and no blocking issues, checked against the code graph (not a self-assessment).
Graphify review — findings
Moves the titleComparison module from utils/music to services/musicManagement and updates all importers and mocks to the new path. Adds barrel-level tests covering delegation through the facade to the real TitleComparisonService, plus cache growth, case-insensitive keying, per-instance isolation, and custom-threshold behaviour, mocking @lucky/shared/utils so the constructor's cleanup interval doesn't leak into the jest worker.
No blocking issues surfaced. 1 lower-confidence candidate did not survive cross-model review.
Analysis details — impact, health, verification
Impact & health
Graphify review
Impact — 94 functions depend on the 69 functions this change touches.
Health — this change adds coupling hotspots:
- new:
handleEvents()— 5 callers, 15 callees - new:
createQueueEmbed()— 4 callers, 6 callees - new:
routeButtonAction()— 1 callers, 10 callees - new:
runInteraction()— 1 callers, 7 callees
Verification — 94 functions in the blast radius were not formally verified this run (proofs are advisory here).
Health delta baseline: last indexed commit 2adc282, 1 commit(s) behind this PR's base.
Gate & verification
graphify gate
PASS — objectively clean (no health regressions, tests not run — proofs not run this pass (advisory)). Grounded, not self-assessed.
Advisory (not blocking):
- verification_scope: 82 function(s) in the blast radius were not formally verified this run
· 4 more finding(s) on lines outside this diff (see the check run).
There was a problem hiding this comment.
All reported issues were addressed across 1 file (changes from recent commits).
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
…ucasSantana-Dev/Lucky into refactor/titlecomparison-into-services
There was a problem hiding this comment.
Graphify reviewed this change.
Looks safe to merge — no coupling regressions and no blocking issues, checked against the code graph (not a self-assessment).
Graphify review — findings
Moves the titleComparison module from utils/music into services/musicManagement alongside its sibling trackProcessor, updating all importers (queueDisplay, trackUtils) to the new location. Adds a barrel spec that exercises extractArtistTitle, isSimilarTitle, calculateSimilarity, and the per-instance/singleton cache against the real service, mocking @lucky/shared/utils so the constructor's cache-cleanup interval doesn't keep the jest worker alive.
No blocking issues surfaced. 1 lower-confidence candidate did not survive cross-model review.
Analysis details — impact, health, verification
Impact & health
Graphify review
Impact — 94 functions depend on the 69 functions this change touches.
Health — this change adds coupling hotspots:
- new:
handleEvents()— 5 callers, 15 callees - new:
createQueueEmbed()— 4 callers, 6 callees - new:
routeButtonAction()— 1 callers, 10 callees - new:
runInteraction()— 1 callers, 7 callees
Verification — 94 functions in the blast radius were not formally verified this run (proofs are advisory here).
Gate & verification
graphify gate
PASS — objectively clean (no health regressions, tests not run — proofs not run this pass (advisory)). Grounded, not self-assessed.
Advisory (not blocking):
- verification_scope: 82 function(s) in the blast radius were not formally verified this run
· 4 more finding(s) on lines outside this diff (see the check run).
|
1 participant
Third and final slice of #1991. Closes it, once #2354 also lands.
TitleComparisonServiceholdsprivate readonly cache: Map<string, ArtistTitle>, so it is a stateful service rather than a util. Last of the five classes #1991 enumerated:TrackCacheManagerSearchEngineManager,ProviderHealthService,YouTubeErrorAnalyzerTitleComparisonServiceReferences rewritten
Eleven, across three shapes:
'../../../../utils/music/titleComparison'queueDisplay.ts+ spec'../../../utils/music/titleComparison'trackUtils(3 files)'../../../src/utils/music/titleComparison/service'tests/utils/music/titleComparison.test.tsThat last one is a
jest.mock()literal in thetests/tree. #2354 hit exactly this and the suite caught it rather than my scan, so this slice scanned both trees up front.The eight from trackUtils collapse back to
'../titleComparison'They were relative siblings originally. #2353 moved
trackUtilsinto services, which turned them into'../../../utils/music/titleComparison'. Now that both live underservices/musicManagement/, they are siblings again and the specifier returns to its original form. Worth noting so a reviewer does not read it as an unrelated change.One outward dependency needed the reverse rewrite
The moved code sits one level deeper relative to
utils/, so:in
service.tsand in thejest.mock()literal inservice.spec.ts.Independence from #2354
This slice touches neither
search/noryoutubeErrorHandler/, and its only prerequisite (#2353, for thetrackUtilslocation) is already onmain. So it is branched offmainrather than stacked on #2354, and the two can merge in either order.Verification
packages/botfull suite: 273 suites passed, 3588 tests passed, 1 skipped, exactly the pre-move baseline (pure move, no test added or removed)npx tsc --noEmit: cleannpx madge --circular --extensions ts packages/bot/src:No circular dependency found!npm run build --workspace=packages/bot: succeedssrc/andtests/: no remaining references🤖 Generated with Claude Code
Summary by cubic
Moves
TitleComparisonServiceout ofutilsintoservicesbecause it maintains a private cache and behaves as a stateful service. This is the last slice of the #1991 refactor and closes it; no behavior changes.Refactors
queueDisplay,trackUtils, and ajest.mock()in the test tree.trackUtilsimports revert to the sibling path../titleComparisonnow that both live underservices/musicManagement.../../misc/stringUtilsbecomes../../../utils/misc/stringUtilsinservice.tsand its spec.Tests
extractArtistTitledoesn't split "artist - title" because pattern lists ship empty; the gap is filed as bug(bot): applyPatterns is a stub, so title extraction always returns artist Unknown #2356.Written for commit 6bd85bd. Summary will update on new commits.