apply claude code fixes by andmoredev · Pull Request #37 · nullchecktv/stream-post-processor

andmoredev · 2026-02-18T19:10:16Z

No description provided.

Copilot

Pull request overview

This pull request applies Claude code fixes to improve timestamp handling and transcript extraction. The changes add millisecond support to time parsing functions and implement SRT-based transcript extraction for clip retrieval with fallback to AI-stored text.

Changes:

Added millisecond support to timeToSeconds functions in video-processing.mjs and transcripts.mjs to handle formats like HH:MM:SS,mmm
Implemented SRT file loading and time-range-based transcript extraction in get-clip.mjs with fallback to stored segment text
Updated AI agent prompt examples to show millisecond timestamps and improved JSON formatting
Added simplified architecture diagram to documentation

Reviewed changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated 9 comments.

Show a summary per file

File	Description
functions/utils/video-processing.mjs	Enhanced timeToSeconds to parse milliseconds with comma separator
functions/utils/transcripts.mjs	Added null-safe millisecond handling to timeToSeconds
functions/clips/get-clip.mjs	Implemented SRT file loading from S3 and time-range-based transcript extraction with fallback logic
functions/agents/clip-detector.mjs	Updated prompt examples to include millisecond timestamps and improved JSON structure
docs/architecture-diagrams.md	Added new simplified architecture diagram in Mermaid format

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-02-18T19:15:46Z

-  const parts = timeStr.split(':').map(part => parseInt(part, 10));
+  const [timePart, msPart] = timeStr.split(',');
+  const parts = timePart.split(':').map(Number);
+  const milliseconds = msPart ? parseInt(msPart) / 1000 : 0;


The milliseconds parsing logic doesn't handle base 10 correctly. The parseInt function should explicitly specify base 10 as the second parameter. Without it, strings with leading zeros like "060" might be parsed incorrectly in some JavaScript environments. Use parseInt(msPart, 10) instead of parseInt(msPart).

Suggested change

const milliseconds = msPart ? parseInt(msPart) / 1000 : 0;

const milliseconds = msPart ? parseInt(msPart, 10) / 1000 : 0;

Copilot · 2026-02-18T19:15:46Z

-    return hours * 3600 + minutes * 60 + seconds;
+    return hours * 3600 + minutes * 60 + seconds + milliseconds;
  } else {
    throw new Error('Time string must be in HH:MM:SS or MM:SS format');


The error message states 'Time string must be in HH:MM:SS or MM:SS format' but this is now outdated. The function now supports milliseconds with the format HH:MM:SS,mmm or MM:SS,mmm. The error message should be updated to reflect this: 'Time string must be in HH:MM:SS, MM:SS, HH:MM:SS,mmm, or MM:SS,mmm format'.

Suggested change

throw new Error('Time string must be in HH:MM:SS or MM:SS format');

throw new Error('Time string must be in HH:MM:SS, MM:SS, HH:MM:SS,mmm, or MM:SS,mmm format');

Copilot · 2026-02-18T19:15:46Z

+  const [timePart, msPart] = timeStr.split(',');
+  const parts = timePart.split(':').map(Number);
+  const milliseconds = msPart ? parseInt(msPart) / 1000 : 0;


The new millisecond support in timeToSeconds functions is not adequately tested. The existing tests in tests/unit/utils/video-processing.test.js only test the HH:MM:SS and MM:SS formats without milliseconds. Tests should be added to verify parsing of HH:MM:SS,mmm and MM:SS,mmm formats, including edge cases like '00:00:00,000', '01:30:45,500', and '10:00,100'.

@copilot open a new pull request to apply changes based on this feedback

Copilot · 2026-02-18T19:15:47Z

+          const segStart = timeToSeconds(segment.startTime);
+          const segEnd = timeToSeconds(segment.endTime);
+
+          const relevantEntries = srtEntries.filter(entry => {
+            const entryStart = timeToSeconds(entry.startTime);
+            const entryEnd = timeToSeconds(entry.endTime);
+            return entryStart < segEnd && entryEnd > segStart;
+          });
+
+          if (relevantEntries.length > 0) {
+            // Use the full SRT text (preserves per-entry speaker labels where present)
+            const text = relevantEntries.map(e => e.text).join(' ');
+            return text;


The SRT loading and transcript extraction logic doesn't handle potential errors from timeToSeconds if the segment timestamps don't match the expected format. If segments have timestamps without milliseconds (HH:MM:SS format) but srtEntries have timestamps with milliseconds (HH:MM:SS,mmm format from SRT files), the timeToSeconds function will work for both. However, if there's a format mismatch or invalid timestamp, the function will throw an error that's not caught here, causing the entire handler to fail. Consider wrapping the timeToSeconds calls in a try-catch block or adding validation to ensure consistent formats.

Suggested change

const segStart = timeToSeconds(segment.startTime);

const segEnd = timeToSeconds(segment.endTime);

const relevantEntries = srtEntries.filter(entry => {

const entryStart = timeToSeconds(entry.startTime);

const entryEnd = timeToSeconds(entry.endTime);

return entryStart < segEnd && entryEnd > segStart;

});

if (relevantEntries.length > 0) {

// Use the full SRT text (preserves per-entry speaker labels where present)

const text = relevantEntries.map(e => e.text).join(' ');

return text;

try {

const segStart = timeToSeconds(segment.startTime);

const segEnd = timeToSeconds(segment.endTime);

const relevantEntries = srtEntries.filter(entry => {

const entryStart = timeToSeconds(entry.startTime);

const entryEnd = timeToSeconds(entry.endTime);

return entryStart < segEnd && entryEnd > segStart;

});

if (relevantEntries.length > 0) {

// Use the full SRT text (preserves per-entry speaker labels where present)

const text = relevantEntries.map(e => e.text).join(' ');

return text;

}

} catch (err) {

logger.warn('Failed to parse timestamps for SRT transcript extraction, falling back to stored text', {

error: err.message,

episodeId,

tenantId,

clipId,

segment

});

Copilot · 2026-02-18T19:15:47Z

+        if (srtEntries.length > 0) {
+          const segStart = timeToSeconds(segment.startTime);
+          const segEnd = timeToSeconds(segment.endTime);
+
+          const relevantEntries = srtEntries.filter(entry => {
+            const entryStart = timeToSeconds(entry.startTime);
+            const entryEnd = timeToSeconds(entry.endTime);
+            return entryStart < segEnd && entryEnd > segStart;
+          });


The SRT file is loaded from S3 on every get-clip request, and then every segment's time range is compared against all SRT entries using filter operations. For clips with multiple segments and large SRT files (hundreds of entries), this results in O(segments × entries) time complexity. Consider caching the parsed SRT entries or implementing a more efficient lookup strategy (e.g., building a time-based index). Alternatively, document that this is acceptable for the expected scale of operations.

@copilot open a new pull request to apply changes based on this feedback

Copilot · 2026-02-18T19:15:47Z

  const [time, ms] = timeStr.split(',');
  const [hours, minutes, seconds] = time.split(':').map(Number);
-  return hours * 3600 + minutes * 60 + seconds + parseInt(ms) / 1000;
+  return hours * 3600 + minutes * 60 + seconds + (ms ? parseInt(ms) / 1000 : 0);


The parseInt function should explicitly specify base 10 as the second parameter to ensure consistent parsing. Use parseInt(ms, 10) instead of parseInt(ms) to avoid potential issues with strings that have leading zeros.

@copilot open a new pull request to apply changes based on this feedback

Copilot · 2026-02-18T19:15:47Z

+    {
+      "startTime": "00:14:32,000",
+      "endTime": "00:15:18,500",
+      "speaker": "Allen",
+      "order": 1,
+      "transcript": "Did you know agents could do this? I was blown away the first time I saw it work end-to-end. You basically hand it a tool and it figures out the rest — no scaffolding, no hand-holding. It just goes. And the crazy part is it gets it right most of the time."
+    },
+    {
+      "startTime": "00:41:01,000",
+      "endTime": "00:41:05,200",
+      "speaker": "Andres",
+      "order": 2,
+      "transcript": "No I didn't, but now we can use it in production."
+    }


The example timestamps in the prompt now include milliseconds (e.g., "00:14:32,000"), but there's an inconsistency with the broader codebase. The global TimestampSchema in schemas/common.mjs still only accepts HH:MM:SS format without milliseconds (/^\d{2}:\d{2}:\d{2}$/), and tests explicitly verify this. While the create-clips tool schema was updated locally to accept milliseconds, this creates a potential validation mismatch. If segments are stored with millisecond timestamps, they may fail validation in other parts of the system that use the global schema. Consider updating the global TimestampSchema or documenting why some timestamps have milliseconds while others don't.

@copilot open a new pull request to apply changes based on this feedback

Copilot · 2026-02-18T19:45:00Z

@andmoredev I've opened a new pull request, #38, to work on those changes. Once the pull request is ready, I'll request review from you.

Co-authored-by: andmoredev <33256364+andmoredev@users.noreply.github.com>

Fix inconsistent speaker label handling in clip transcripts

Copilot · 2026-02-18T19:54:20Z

@andmoredev I've opened a new pull request, #39, to work on those changes. Once the pull request is ready, I'll request review from you.

Copilot · 2026-02-18T19:54:45Z

@andmoredev I've opened a new pull request, #40, to work on those changes. Once the pull request is ready, I'll request review from you.

Copilot · 2026-02-18T19:54:53Z

@andmoredev I've opened a new pull request, #41, to work on those changes. Once the pull request is ready, I'll request review from you.

Copilot · 2026-02-18T19:54:56Z

@andmoredev I've opened a new pull request, #42, to work on those changes. Once the pull request is ready, I'll request review from you.

apply claude code fixes

f6db436

Copilot AI review requested due to automatic review settings February 18, 2026 19:10

Copilot started reviewing on behalf of andmoredev February 18, 2026 19:10 View session

Copilot AI reviewed Feb 18, 2026

View reviewed changes

andmoredev and others added 2 commits February 18, 2026 12:20

let's see

e1f6d76

Initial plan

06f34c8

Copilot AI mentioned this pull request Feb 18, 2026

Fix inconsistent speaker label handling in clip transcripts #38

Merged

Copilot AI and others added 2 commits February 18, 2026 19:47

Fix: Always prepend speaker label to SRT transcript text

c7c1405

Co-authored-by: andmoredev <33256364+andmoredev@users.noreply.github.com>

Merge pull request #38 from nullchecktv/copilot/sub-pr-37

be4c1b6

Fix inconsistent speaker label handling in clip transcripts

Copilot AI mentioned this pull request Feb 18, 2026

Add test coverage for millisecond support in timeToSeconds #39

Draft

Copilot AI mentioned this pull request Feb 18, 2026

Optimize get-clip SRT lookup with LRU cache and time-based indexing #40

Draft

Copilot AI mentioned this pull request Feb 18, 2026

Add explicit radix to parseInt calls #41

Draft

Copilot AI mentioned this pull request Feb 18, 2026

Fix timestamp schema inconsistency between global validation and tool schemas #42

Draft

andmoredev merged commit c24a551 into main Feb 18, 2026
5 of 6 checks passed

	const milliseconds = msPart ? parseInt(msPart) / 1000 : 0;
	const milliseconds = msPart ? parseInt(msPart, 10) / 1000 : 0;

	throw new Error('Time string must be in HH:MM:SS or MM:SS format');
	throw new Error('Time string must be in HH:MM:SS, MM:SS, HH:MM:SS,mmm, or MM:SS,mmm format');

Uh oh!

Conversation

andmoredev commented Feb 18, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Uh oh!

Copilot AI Feb 18, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 18, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 18, 2026

Choose a reason for hiding this comment

Uh oh!

andmoredev Feb 18, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 18, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 18, 2026

Choose a reason for hiding this comment

Uh oh!

andmoredev Feb 18, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 18, 2026

Choose a reason for hiding this comment

Uh oh!

andmoredev Feb 18, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 18, 2026

Choose a reason for hiding this comment

Uh oh!

andmoredev Feb 18, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI commented Feb 18, 2026

Uh oh!

Copilot AI commented Feb 18, 2026

Uh oh!

Copilot AI commented Feb 18, 2026

Uh oh!

Copilot AI commented Feb 18, 2026

Uh oh!

Copilot AI commented Feb 18, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants