FEAT: use content path instead of history for checking if usenet NZB is in library #553
Conversation
FIX use content path instead of history (hard-coded Movies category)
FIXED hard-coded category: fetch Movies and TV in parallel
* Update usenet-stream-base.ts * Update usenet-stream-base.ts * Update usenet-stream-base.ts * Update usenet-stream-base.ts * Update usenet-stream-base.ts
WalkthroughThe pull request modifies three core components: changes the NZB listing mechanism from history API to WebDAV aggregation, updates regex patterns for HDR/DV visual tags and German language detection, and adds cache-aware deduplication logic to distinguish between cached and uncached streams. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/core/src/debrid/usenet-stream-base.ts (1)
604-647: Library detection is unreliable:namenever set and hash–hash comparison missingTwo issues combine here and will cause
checkNzbsto miss items that are already in the library:
In
listNzbs(Line 618 onwards), the mappedDebridDownloadobjects never set anamefield – onlyhash,sizeandfiles. As a result, incheckNzbs(Line 690), bothnzb.name === nandnzb.name === hwill always befalsefor entries coming fromlistNzbs.The core comparison you would normally expect –
nzb.hash === h(library hash vs incoming hash) – is not present. The current predicate:(nzb) => nzb.name === n || nzb.name === h || nzb.hash === nnever compares
nzb.hashtoh, so direct hash matches are impossible. The only way to match is if the libraryhashhappens to equal the incomingname, which is quite brittle and likely unintended.This will under-report
library: trueincheckNzbs, and can lead to repeat NZB submissions even when a corresponding content directory already exists.A minimal, consistent fix is:
- Set both
nameandhashfrom the folder basename inlistNzbs, so the folder name can be matched against either incoming field.- Restore a proper
hash === hashcomparison incheckNzbs, while still allowing for the “swapped” cases if you need them.Example diff:
- // Combine the results - const combinedContents = [...(moviesContents as FileStat[]), ...(tvContents as FileStat[])]; - - // Map to NZB objects - const nzbs = combinedContents.map((item, index) => ({ - id: index, - status: 'cached' as const, - hash: item.basename, - size: item.size, - files: [], - })); + // Combine the results + const combinedContents = [ + ...(moviesContents as FileStat[]), + ...(tvContents as FileStat[]), + ]; + + // Treat each directory under Movies/TV as a library entry keyed by its folder name + const nzbs = combinedContents + .filter((item) => item.type === 'directory') + .map((item, index) => ({ + id: index, + status: 'cached' as const, + name: item.basename, + hash: item.basename, + size: item.size, + files: [], + })); @@ - const libraryNzb = libraryNzbs.find( - (nzb) => nzb.name === n || nzb.name === h || nzb.hash === n - ); + const libraryNzb = libraryNzbs.find((nzb) => + // Compare both stored fields (name/hash) against both inputs (name/hash), + // so we are robust to how the folder name was derived. + (n && (nzb.name === n || nzb.hash === n)) || + (h && (nzb.name === h || nzb.hash === h)) + );This keeps the new “content path–driven” behaviour but makes
checkNzbsreliably detect existing NZBs by their folder name, regardless of whether callers supply that asnameorhash.Also applies to: 690-699
🧹 Nitpick comments (1)
packages/core/src/debrid/usenet-stream-base.ts (1)
631-640: Consider removing the old history-based implementation once migration is stableThe previous history-based
listNzbsimplementation is now fully commented out (Lines 631–640). Once you are confident the WebDAV/content-path approach is behaving as expected in production, consider deleting this block to reduce noise and avoid future confusion about which source of truth is actually used.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
packages/core/src/debrid/usenet-stream-base.ts(2 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
packages/core/src/debrid/usenet-stream-base.ts (1)
packages/core/src/utils/logger.ts (1)
getTimeTakenSincePoint(152-159)
|
@Viren070 could you please have a look at this? |
There was a problem hiding this comment.
Actionable comments posted: 3
📜 Review details
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
packages/core/src/debrid/usenet-stream-base.ts
🧰 Additional context used
🧬 Code graph analysis (1)
packages/core/src/debrid/usenet-stream-base.ts (1)
packages/core/src/utils/logger.ts (1)
getTimeTakenSincePoint(152-159)
| // Construct paths | ||
| const moviesPath = `${this.getContentPathPrefix()}/Movies`; | ||
| const tvPath = `${this.getContentPathPrefix()}/TV`; | ||
|
|
||
| // Fetch both directories in parallel | ||
| const [moviesContents, tvContents] = await Promise.all([ | ||
| this.webdavClient.getDirectoryContents(moviesPath), | ||
| this.webdavClient.getDirectoryContents(tvPath), | ||
| ]); |
There was a problem hiding this comment.
Add error handling for WebDAV directory fetching.
The parallel Promise.all will fail entirely if either the Movies or TV directory doesn't exist or encounters a permissions issue. This could break the library checking functionality completely.
🔎 Proposed fix to handle missing or inaccessible directories
// Construct paths
const moviesPath = `${this.getContentPathPrefix()}/Movies`;
const tvPath = `${this.getContentPathPrefix()}/TV`;
// Fetch both directories in parallel
-const [moviesContents, tvContents] = await Promise.all([
- this.webdavClient.getDirectoryContents(moviesPath),
- this.webdavClient.getDirectoryContents(tvPath),
-]);
+const [moviesContents, tvContents] = await Promise.all([
+ this.webdavClient.getDirectoryContents(moviesPath).catch((error) => {
+ this.serviceLogger.warn(`Failed to list Movies directory`, {
+ path: moviesPath,
+ error: (error as Error).message,
+ });
+ return [];
+ }),
+ this.webdavClient.getDirectoryContents(tvPath).catch((error) => {
+ this.serviceLogger.warn(`Failed to list TV directory`, {
+ path: tvPath,
+ error: (error as Error).message,
+ });
+ return [];
+ }),
+]);📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| // Construct paths | |
| const moviesPath = `${this.getContentPathPrefix()}/Movies`; | |
| const tvPath = `${this.getContentPathPrefix()}/TV`; | |
| // Fetch both directories in parallel | |
| const [moviesContents, tvContents] = await Promise.all([ | |
| this.webdavClient.getDirectoryContents(moviesPath), | |
| this.webdavClient.getDirectoryContents(tvPath), | |
| ]); | |
| // Construct paths | |
| const moviesPath = `${this.getContentPathPrefix()}/Movies`; | |
| const tvPath = `${this.getContentPathPrefix()}/TV`; | |
| // Fetch both directories in parallel | |
| const [moviesContents, tvContents] = await Promise.all([ | |
| this.webdavClient.getDirectoryContents(moviesPath).catch((error) => { | |
| this.serviceLogger.warn(`Failed to list Movies directory`, { | |
| path: moviesPath, | |
| error: (error as Error).message, | |
| }); | |
| return []; | |
| }), | |
| this.webdavClient.getDirectoryContents(tvPath).catch((error) => { | |
| this.serviceLogger.warn(`Failed to list TV directory`, { | |
| path: tvPath, | |
| error: (error as Error).message, | |
| }); | |
| return []; | |
| }), | |
| ]); |
| // const history = await this.api.history(); | ||
| // const nzbs: DebridDownload[] = history.slots.map((slot, index) => ({ | ||
| // id: index, | ||
| // status: slot.status !== 'failed' ? 'cached' : 'failed', | ||
| // name: slot.name, | ||
| // })); | ||
| // this.serviceLogger.debug(`Listed NZBs from history`, { | ||
| // count: nzbs.length, | ||
| // time: getTimeTakenSincePoint(start), | ||
| // }); |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Remove commented-out code.
The commented history API implementation should be removed rather than left in the codebase. Version control preserves the history if needed.
🔎 Proposed cleanup
- // const history = await this.api.history();
- // const nzbs: DebridDownload[] = history.slots.map((slot, index) => ({
- // id: index,
- // status: slot.status !== 'failed' ? 'cached' : 'failed',
- // name: slot.name,
- // }));
- // this.serviceLogger.debug(`Listed NZBs from history`, {
- // count: nzbs.length,
- // time: getTimeTakenSincePoint(start),
- // });
-
await UsenetStreamService.libraryCache.set(📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| // const history = await this.api.history(); | |
| // const nzbs: DebridDownload[] = history.slots.map((slot, index) => ({ | |
| // id: index, | |
| // status: slot.status !== 'failed' ? 'cached' : 'failed', | |
| // name: slot.name, | |
| // })); | |
| // this.serviceLogger.debug(`Listed NZBs from history`, { | |
| // count: nzbs.length, | |
| // time: getTimeTakenSincePoint(start), | |
| // }); | |
| await UsenetStreamService.libraryCache.set( |
🤖 Prompt for AI Agents
In packages/core/src/debrid/usenet-stream-base.ts around lines 631 to 640,
remove the commented-out history API block (the commented const history = ...
nzbs mapping and serviceLogger.debug call); delete those commented lines
entirely and, after removal, run a quick search to clean up any now-unused
variables or imports referenced only by that commented snippet (e.g., start,
history, nzbs) and remove them as well.
| return nzbs.map(({ hash: h, name: n }, index) => { | ||
| const libraryNzb = libraryNzbs.find( | ||
| (nzb) => nzb.name === n || nzb.name === h | ||
| (nzb) => nzb.hash === n | ||
| ); |
There was a problem hiding this comment.
Critical: Incorrect hash comparison in library lookup.
The comparison on Line 691 uses nzb.hash === n, but n is the name parameter (from the destructuring on Line 689), not the hash. This should compare nzb.hash === h to properly match by hash.
This breaks the library lookup functionality described in the PR objective ("more precise library lookup using only the hash").
🔎 Proposed fix
return nzbs.map(({ hash: h, name: n }, index) => {
const libraryNzb = libraryNzbs.find(
- (nzb) => nzb.hash === n
+ (nzb) => nzb.hash === h
);
return {📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| return nzbs.map(({ hash: h, name: n }, index) => { | |
| const libraryNzb = libraryNzbs.find( | |
| (nzb) => nzb.name === n || nzb.name === h | |
| (nzb) => nzb.hash === n | |
| ); | |
| return nzbs.map(({ hash: h, name: n }, index) => { | |
| const libraryNzb = libraryNzbs.find( | |
| (nzb) => nzb.hash === h | |
| ); |
🤖 Prompt for AI Agents
In packages/core/src/debrid/usenet-stream-base.ts around lines 689 to 692, the
library lookup compares nzb.hash to the destructured name variable (`n`) instead
of the hash alias (`h`); update the find predicate to compare nzb.hash === h so
it matches by hash as intended, then run tests/linters to confirm no other
places use the wrong variable.
DV+HDR regex patterns and update German regex
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
packages/core/src/parser/regex.ts
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-09-14T16:46:18.507Z
Learnt from: DavidGracias
Repo: Viren070/AIOStreams PR: 375
File: packages/core/src/formatters/base.ts:0-0
Timestamp: 2025-09-14T16:46:18.507Z
Learning: In packages/core/src/formatters/base.ts, the modifier definitions intentionally include regex patterns (like "join('.*?')", "$.*?", "^.*?") that require pattern matching functionality. The buildModifierRegexPattern() method should not fully escape all regex metacharacters as this would break the intended regex matching behavior within modifier definitions.
Applied to files:
packages/core/src/parser/regex.ts
🔇 Additional comments (1)
packages/core/src/parser/regex.ts (1)
147-147: The German regex simplification has no practical impact on content detection. Whilst the pattern was simplified by removing(land)?, a search of the codebase confirms "deutschland" is not used anywhere, and the regex still matches the primary German language identifiers ("deutsch", "german", "ger", "deu").
Add new visual tags for HDR formats
Rename regex key for HDR10Plus
Removed deprecated visual tags from the constants.
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
packages/core/src/parser/regex.ts
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-09-14T16:46:18.507Z
Learnt from: DavidGracias
Repo: Viren070/AIOStreams PR: 375
File: packages/core/src/formatters/base.ts:0-0
Timestamp: 2025-09-14T16:46:18.507Z
Learning: In packages/core/src/formatters/base.ts, the modifier definitions intentionally include regex patterns (like "join('.*?')", "$.*?", "^.*?") that require pattern matching functionality. The buildModifierRegexPattern() method should not fully escape all regex metacharacters as this would break the intended regex matching behavior within modifier definitions.
Applied to files:
packages/core/src/parser/regex.ts
🔇 Additional comments (2)
packages/core/src/parser/regex.ts (2)
188-188: LGTM!Adding a newline at end of file is standard practice and improves version control consistency.
83-87: The regex patterns correctly collect overlapping matches as intended.Both the HDR10+, HDR10, and HDR patterns (lines 83–85) and the DV pattern (line 87) are designed to match overlapping input strings such as "DV+HDR10+". The
matchMultiplePatternsfunction at line 96 inpackages/core/src/parser/file.tsiterates through all patterns and returns all matching tags as an array rather than selecting a single tag. This means filenames matching "DV+HDR10+" correctly produce both "HDR10+" and "DV" tags, which is the intended behaviour for a tag collector that identifies all applicable visual tags. No precedence handling is needed because the parser returns all matches, not a single tag.
| Spanish: createLanguageRegex('spanish|spa|esp'), | ||
| French: createLanguageRegex('french|fra|fr|vf|vff|vfi|vf2|vfq|truefrench'), | ||
| German: createLanguageRegex('deu(tsch)?(land)?|ger(man)?'), | ||
| German: createLanguageRegex('deu(tsch)?|ger(man)?'), |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Step 1: View the full context of line 146 and surrounding code in regex.ts
echo "=== Full context of regex.ts around line 146 ==="
cat -n packages/core/src/parser/regex.ts | sed -n '140,160p'
echo ""
echo "=== Looking for language regex definitions ==="
cat -n packages/core/src/parser/regex.ts | grep -A 5 -B 5 "German\|createLanguageRegex" | head -40Repository: Viren070/AIOStreams
Length of output: 3397
🏁 Script executed:
#!/bin/bash
# Step 2: Check git history to see what the previous pattern was
echo "=== Recent changes to regex.ts ==="
git log --oneline -n 20 -- packages/core/src/parser/regex.ts
echo ""
echo "=== Detailed diff of recent changes (if available) ==="
git diff HEAD~1 HEAD -- packages/core/src/parser/regex.ts | grep -A 10 -B 10 "German\|deu\|ger" || echo "No recent changes to German pattern found"Repository: Viren070/AIOStreams
Length of output: 296
🏁 Script executed:
#!/bin/bash
# Step 3: Search for test cases with German language variations
echo "=== Test files containing German language ==="
find . -type f \( -name "*.test.ts" -o -name "*.spec.ts" -o -name "*test*.ts" \) -exec rg -l "german|deutsch|ger\b|deu\b" {} \;
echo ""
echo "=== Content of test files with German patterns ==="
find . -type f \( -name "*.test.ts" -o -name "*.spec.ts" -o -name "*test*.ts" \) -exec rg -A 3 -B 3 "germany|german" {} + 2>/dev/null | head -60Repository: Viren070/AIOStreams
Length of output: 160
🏁 Script executed:
#!/bin/bash
# Step 4: Check for "germany" specifically in the codebase
echo "=== Search for 'germany' in test files and fixtures ==="
rg -i "germany" --type ts --type json 2>/dev/null | head -30
echo ""
echo "=== Check for German language test fixtures ==="
fd -e json -o -e ts | xargs rg -l "deu|deutsch|german" 2>/dev/null | grep -i "test\|fixture\|example" | head -20Repository: Viren070/AIOStreams
Length of output: 727
No test coverage exists for German language parsing; recommend adding test cases to prevent regressions.
The German language pattern deu(tsch)?|ger(man)? matches standard language identifiers ("deutsch", "german", "deu", "ger"), but there are no test cases in the codebase to verify it. "Germany" appears only in configuration presets, not in release name test data. Consider adding test cases for German language variants to prevent future regressions.
Fix deduplication logic to include cache class prefix for mixing cached and uncached streams.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
packages/core/src/streams/deduplicator.ts (1)
60-71: Consider extracting the cacheClass logic into a helper function.The logic to determine whether a stream is cached, uncached, or other (lines 62-70) is duplicated at lines 156-163. Extracting this into a helper function would improve maintainability and ensure consistency if the classification logic needs to change.
🔎 Proposed refactor to extract helper function
Add a helper method to the class:
private getCacheClass(stream: ParsedStream): string { if ( (stream.type === 'debrid' || stream.type === 'usenet' || stream.type === 'stremio-usenet') && stream.service ) { return stream.service.cached ? 'cached' : 'uncached'; } return 'other'; }Then replace lines 62-70 with:
- // FIX dedup mixing cached and uncached with a "cache-class" prefix - const cacheClass = - (stream.type === 'debrid' || - stream.type === 'usenet' || - stream.type === 'stremio-usenet') && - stream.service - ? stream.service.cached - ? 'cached' - : 'uncached' - : 'other'; + // FIX dedup mixing cached and uncached with a "cache-class" prefix + const cacheClass = this.getCacheClass(stream);And replace lines 156-163 with:
- let type = stream.type as string; - if ( - (type === 'debrid' || - type === 'usenet' || - type === 'stremio-usenet') && - stream.service - ) { - type = stream.service.cached ? 'cached' : 'uncached'; - } + let type = this.getCacheClass(stream);
📜 Review details
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
packages/core/src/streams/deduplicator.ts
🔇 Additional comments (3)
packages/core/src/streams/deduplicator.ts (3)
77-88: LGTM! Filename deduplication key correctly prefixed with cacheClass.The addition of the cacheClass prefix to the filename deduplication key ensures that cached and uncached streams are not incorrectly grouped together during deduplication.
94-97: LGTM! InfoHash deduplication key correctly prefixed with cacheClass.The cacheClass prefix is properly applied to the infoHash-based deduplication key, maintaining consistency with the filename key changes.
99-110: LGTM! SmartDetect deduplication key correctly prefixed with cacheClass.The cacheClass prefix completes the fix across all three deduplication key types (filename, infoHash, and smartDetect), ensuring cached and uncached streams are properly separated during deduplication.
1 participant
FEAT: use content path instead of history for checking if usenet NZB is in library
Summary by CodeRabbit
Release Notes
Bug Fixes
Improvements
✏️ Tip: You can customize this high-level summary in your review settings.