Skip to content

fix: clips not found with no extensions#311

Merged
justandras merged 3 commits into
Sofie-Automation:mainfrom
bbc:fix/clips-not-found-with-no-extensions
May 20, 2026
Merged

fix: clips not found with no extensions#311
justandras merged 3 commits into
Sofie-Automation:mainfrom
bbc:fix/clips-not-found-with-no-extensions

Conversation

@justandras
Copy link
Copy Markdown
Member

@justandras justandras commented May 19, 2026

About Me

This pull request is posted on behalf of the BBC.

Type of Contribution

This is a: Bug fix

Current Behavior

When MATCH_FILENAMES_WITHOUT_EXTENSION=1 is used full path matches are ignored.

unset MATCH_FILENAMES_WITHOUT_EXTENSION

  • fi.le.mp4 finds fi.le.mp4
  • fi.le doesn't find fi.le.mp4
  • fi.le finds fi.le

MATCH_FILENAMES_WITHOUT_EXTENSION=1

  • fi.le.mp4 doesn't find fi.le.mp4
  • fi.le finds fi.le.mp4
  • fi.le doesn't find fi.le

New Behavior

Full paths now resolve to the correct files.

When MATCH_FILENAMES_WITHOUT_EXTENSION=1 is used full path matches are ignored.

unset MATCH_FILENAMES_WITHOUT_EXTENSION

  • fi.le.mp4 finds fi.le.mp4
  • fi.le doesn't find fi.le.mp4
  • fi.le finds fi.le

MATCH_FILENAMES_WITHOUT_EXTENSION=1

  • fi.le.mp4 finds fi.le.mp4
  • fi.le finds fi.le.mp4
  • fi.le finds fi.le

Testing Instructions

Test the cases listed above my changing the expected path in NRCS and renaming the files in the directory observed by package manager.

Status

  • PR is ready to be reviewed.
  • The functionality has been tested by the author.
  • Relevant unit tests has been added / updated.
  • Relevant documentation (code comments, system documentation) has been added / updated.

@justandras justandras requested review from nytamin and rjmunro May 19, 2026 15:15
@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented May 19, 2026

Review Change Stack

Walkthrough

The PR extends resolveFileWithoutExtension in filePath.ts to match exact baseline filenames without extensions, returning an empty string for extension in such cases. Three new test cases validate the updated behavior for extensionless files, pre-extended input paths, and scenarios with multiple matching files.

Changes

File Resolution Matching

Layer / File(s) Summary
Exact filename matching and test coverage
shared/packages/api/src/filePath.ts, shared/packages/api/src/__tests__/filePath.spec.ts
The match predicate is expanded to include exact filename matches (f === base) alongside dot-extension prefix matches. Test cases validate resolution of extensionless files, paths that already include extensions, and multiple-match scenarios where both extensionless and extension versions exist.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~8 minutes

Possibly related PRs

Suggested labels

Contribution from BBC

Suggested reviewers

  • nytamin
🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed The title 'fix: clips not found with no extensions' directly addresses the main bug being fixed—clips with no extensions not being found when using MATCH_FILENAMES_WITHOUT_EXTENSION, which is the core issue resolved by this changeset.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
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.
Description check ✅ Passed The PR description clearly explains the bug being fixed (full path matches ignored with MATCH_FILENAMES_WITHOUT_EXTENSION=1), contrasts current vs new behavior with specific examples, and provides testing instructions.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

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.

❤️ Share

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

@justandras justandras self-assigned this May 19, 2026
@justandras justandras added bug Something isn't working Contribution from BBC Contributions sponsored by BBC (bbc.co.uk) labels May 19, 2026
Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

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

🧹 Nitpick comments (1)
shared/packages/api/src/filePath.ts (1)

38-60: ⚡ Quick win

Update JSDoc to document exact-match behavior and empty extension cases.

The JSDoc examples don't cover the new exact-match behavior introduced by the || f === base condition. Consider adding examples showing:

  • When an extensionless file exists (e.g., searching for /path/to/file finds file → returns extension: '')
  • When the input path already includes an extension (e.g., searching for /path/to/file.mp4 finds file.mp4 → returns extension: '')
  • When both extensionless and extension files exist (returns multiple)

This will help users understand when the extension field will be an empty string versus containing an actual extension.

📝 Suggested JSDoc additions
  * `@example`
  * // If looking for /path/to/file and file.tar.gz exists:
  * const result = await resolveFileWithoutExtension('/path/to/file')
  * // Returns: { result: 'found', fullPath: '/path/to/file.tar.gz', extension: '.tar.gz' }
  *
+ * `@example`
+ * // If looking for /path/to/file and an extensionless file exists:
+ * const result = await resolveFileWithoutExtension('/path/to/file')
+ * // Returns: { result: 'found', fullPath: '/path/to/file', extension: '' }
+ *
+ * `@example`
+ * // If the input path already includes the extension:
+ * const result = await resolveFileWithoutExtension('/path/to/file.mp4')
+ * // Returns: { result: 'found', fullPath: '/path/to/file.mp4', extension: '' }
+ *
  * `@example`
  * // If both file.mp4 and file.mov exist:
  * const result = await resolveFileWithoutExtension('/path/to/file')
  * // Returns: { result: 'multiple', matches: ['/path/to/file.mp4', '/path/to/file.mov'] }
🤖 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 `@shared/packages/api/src/filePath.ts` around lines 38 - 60, Update the JSDoc
for resolveFileWithoutExtension to document the exact-match and empty-extension
behavior: add an example showing when an extensionless file exists (searching
'/path/to/file' finds 'file' and returns extension: ''), an example showing when
the input already includes an extension (searching '/path/to/file.mp4' finds
'file.mp4' and returns extension: ''), and an example showing when both
extensionless and extensioned files exist (returns result: 'multiple' with
matches). Mention that the code treats exact filename matches (f === base) as
extensionless and clarify when extension is an empty string vs a dot-prefixed
extension.
🤖 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 `@shared/packages/api/src/filePath.ts`:
- Around line 38-60: Update the JSDoc for resolveFileWithoutExtension to
document the exact-match and empty-extension behavior: add an example showing
when an extensionless file exists (searching '/path/to/file' finds 'file' and
returns extension: ''), an example showing when the input already includes an
extension (searching '/path/to/file.mp4' finds 'file.mp4' and returns extension:
''), and an example showing when both extensionless and extensioned files exist
(returns result: 'multiple' with matches). Mention that the code treats exact
filename matches (f === base) as extensionless and clarify when extension is an
empty string vs a dot-prefixed extension.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: c0ae49cf-604e-484b-8ee6-0514444160f3

📥 Commits

Reviewing files that changed from the base of the PR and between 0ef4e5b and 4bc20bf.

📒 Files selected for processing (2)
  • shared/packages/api/src/__tests__/filePath.spec.ts
  • shared/packages/api/src/filePath.ts

@sonarqubecloud
Copy link
Copy Markdown

@justandras justandras merged commit d00147e into Sofie-Automation:main May 20, 2026
5 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working Contribution from BBC Contributions sponsored by BBC (bbc.co.uk)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants