Skip to content

feat: record-video returns a timeline sheet of the whole recording - #3374

Merged
hatayama merged 3 commits into
feature/record-video-readable-evidencefrom
feature/record-video-timeline-sheet
Oct 21, 2026
Merged

hatayama merged 3 commits into
feature/record-video-readable-evidencefrom
feature/record-video-timeline-sheet

Conversation

@hatayama

@hatayama hatayama commented Oct 21, 2026 •

Copy link
Copy Markdown
Owner

Summary

  • Stopping a recording now also returns a timeline sheet: one PNG of evenly spaced frames from the whole recording, so an agent can see what the recording showed from start to end without opening the video.

User Impact

  • Before: the stop response pointed only at the burst sheet, which covers about a second around one moment and is absent when nothing changed. An agent had no readable overview of a longer or still recording.
  • After: the stop response carries TimelineSheetPath (5 columns, top-left first), TimelineSheetColumns, and TimelineFrames (frame number and seconds of each tile). The sheet is written for any recording with at least one captured frame, including still ones.

Changes

  • Tiles are kept every half second. A recording's length is unknown while it runs, so when the tiles exceed 40, every other one is dropped and the interval doubles; the sheet stays within 40 tiles at a near-even pace and always starts at the first frame. The next tile is due one interval after the last kept one, so a slow Editor that missed a slot does not bunch tiles together.
  • Dropped tiles' arrays are reused, as in the burst buffer.
  • The sheet and its frame list are written beside the video as <stem>_timeline.png and in the sidecar JSON's Timeline.

Verification

  • uloop compile: 0 errors.
  • uloop run-tests (regex over the timeline sampler, session, session analysis, response mapper, burst buffer, sidecar, and last-recording store tests): 72 passed, 0 failed.
  • Mutations caught: not resetting the next-due time after thinning, writing the timeline only from two tiles, and releasing a kept tile's array during thinning each fail a test.
  • Live Editor check: a 12-second Game View recording gave 24 tiles about 0.5 s apart (0.03 s to 11.93 s), 5 columns, in time order from the top-left, with the last slot empty at the bottom-right.

View guided diff

@coderabbitai

coderabbitai Bot commented Oct 21, 2026 •

Copy link
Copy Markdown
Contributor

Review in Change Stack →

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration
  • Configuration used: Repository: hatayama/unity-cli-loop/.coderabbit.yaml
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: c1442d7f-69f0-489c-95c8-62c679f3e755
📥 Commits

Reviewing files that changed from the base of the PR and between 1fb9fe7 and 69389f2.

⛔ Files ignored due to path filters: 2
  • Assets/Tests/Editor/TimelineFrameSamplerTests.cs.meta is excluded by none and included by none
  • Packages/src/Editor/FirstPartyTools/RecordVideo/TimelineFrameSampler.cs.meta is excluded by none and included by none
📒 Files selected for processing: 7
  • Assets/Tests/Editor/RecordVideoResponseAnalysisMapperTests.cs
  • Assets/Tests/Editor/TimelineFrameSamplerTests.cs
  • Assets/Tests/Editor/VideoRecordingSessionAnalysisTests.cs
  • Packages/src/Editor/FirstPartyTools/RecordVideo/RecordVideoResponse.cs
  • Packages/src/Editor/FirstPartyTools/RecordVideo/RecordVideoResponseAnalysisMapper.cs
  • Packages/src/Editor/FirstPartyTools/RecordVideo/RecordingFrameAnalyzer.cs
  • Packages/src/Editor/FirstPartyTools/RecordVideo/TimelineFrameSampler.cs

Included review availability: This review used your included allowance. Your plan provides up to 4 included reviews per hour; 1 remain after this review.

📝 Walkthrough

Recording analysis now samples frames for a timeline sheet, writes the sheet when timeline frames exist, and maps its path, column count, and frame data into the response. Tests cover sampling behavior, sheet generation, and response mapping.

Changes: Timeline sheet generation and response

Layer / File(s) Summary
Timeline frame sampling
Packages/src/Editor/FirstPartyTools/RecordVideo/TimelineFrameSampler.cs, Assets/Tests/Editor/TimelineFrameSamplerTests.cs
TimelineFrameSampler retains frames at scheduled intervals, copies tile data, and thins the stored tiles after the limit is exceeded. Tests cover intervals, thinning, and tile ownership.
Timeline sheet generation
Packages/src/Editor/FirstPartyTools/RecordVideo/RecordingFrameAnalyzer.cs, Assets/Tests/Editor/VideoRecordingSessionAnalysisTests.cs
RecordingFrameAnalyzer adds captured samples to the timeline sampler and writes a five-column sheet when at least one timeline frame exists. Tests cover no-frame, one-frame, and static-frame recordings.
Timeline response mapping
Packages/src/Editor/FirstPartyTools/RecordVideo/RecordVideoResponse.cs, Packages/src/Editor/FirstPartyTools/RecordVideo/RecordVideoResponseAnalysisMapper.cs, Assets/Tests/Editor/RecordVideoResponseAnalysisMapperTests.cs
RecordVideoResponse adds optional timeline fields. The mapper copies timeline data when present; tests cover mapping without a burst sheet and the absent-timeline case.

Priority: ⚪ Pending latest changes

Estimated code review effort: 3 (Moderate) | ~20 minutes

Change: Feature

Sequence Diagram(s)

sequenceDiagram
  participant RecordingFrameAnalyzer
  participant TimelineFrameSampler
  participant RecordVideoResponseAnalysisMapper
  RecordingFrameAnalyzer->>TimelineFrameSampler: Add captured tile and sample
  TimelineFrameSampler-->>RecordingFrameAnalyzer: Retain sampled timeline frames
  RecordingFrameAnalyzer->>RecordingFrameAnalyzer: Finish and write sheet when frames exist
  RecordingFrameAnalyzer-->>RecordVideoResponseAnalysisMapper: Provide analysis containing timeline data
  RecordVideoResponseAnalysisMapper->>RecordVideoResponseAnalysisMapper: Map timeline path, columns, and frames
Loading
Merge Risk: ⚪ Minimal · up to 69389

The timeline metadata can pass through the existing response path without a CLI contract change. No merge-blocking issue was identified.

Security architecture risk: 🔵 Low · up to 69389

The timeline uses frames already captured by the recording and does not add capture permissions. Risk is limited to an additional potentially sensitive image and consistency of saved outputs when finalization is interrupted or fails.

Retained concerns
No architecture-level concerns identified.

Security review details

Security Blast Radius

  • inferred — The additional exposure is a readable image spanning sampled recording content, written using the Editor process's existing filesystem authority. Explicit recording paths can resolve outside the project; the timeline is derived beside that path rather than accepting a separate destination. The inspected change expands derivative output, not capture-source selection or execution privileges.

Trust Boundaries and Controls

  • observed — Analysis exceptions clear the session's analysis result, and response mapping exits when that result is absent. Automatic-stop persistence records an empty analysis path after failure, preventing that failure path from advertising a previously existing timeline through recovery.

Resilience and Maintainability Implications

  • inferred — The inspected production API path switches to the main thread before invoking the tool, whose stop branch executes synchronously; capture uses EditorApplication.update. This counters a concurrent capture/stop race on that path. Stop is idempotent, and contained analysis failures still allow texture destruction and host callback/session cleanup.
  • observed — The timeline PNG and analysis JSON are written sequentially without a transactional commit or failure cleanup. This extends the pre-existing burst/sidecar persistence pattern to another file. Interruption or failure can leave partial or stale disk outputs, despite suppression of failed analysis in the response.

Hardening Proposals

  • proposed — If direct filesystem consumers require recording-consistent evidence, consider recording-specific artifact generations, staging sheets before publishing their manifest, and cleanup of abandoned generations. This would strengthen the inherited persistence model rather than remedy a verified security vulnerability.
Pre-merge checks: Passed 4 · Failed 1

Failed Failed checks: 1 warning

Check name Status Explanation Resolution
Docstring Coverage Warning Docstring coverage is 54.55% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 22 functions across 7 files. Write docstrings for the functions missing them to satisfy the coverage threshold.
Passed Passed checks: 4 passed
Check name Status Explanation
Title check Passed The title clearly and concisely describes the main change: record-video now returns a timeline sheet covering the whole recording.
Description check Passed The description directly explains the timeline sheet behavior, response fields, sampling rules, output path, and verification results. It is fully related to the changeset.
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: 💡 1
📝 Generate docstrings: 💡
  • Commit to this branch
  • Create a new PR
🧪 Generate unit tests (beta)
  • Commit to this branch
  • Create a new PR
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

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.

@hatayama
hatayama force-pushed the feature/record-video-timeline-sheet branch from 4f299db to 9881b0c Compare October 21, 2026 06:37
Base automatically changed from feature/record-video-one-shot to feature/record-video-readable-evidence October 21, 2026 06:41
…nown length

The timeline sheet should cover a whole recording, whose length is not
known while it runs. Tiles are kept every half second; when they exceed
40, every other one is dropped and the interval doubles, so the sheet
stays within 40 tiles at a near-even pace. Dropped tiles' arrays are
reused.
Stopping a recording now also writes <stem>_timeline.png (5 columns,
top-left first) with evenly spaced tiles from the whole recording, and
the stop response carries TimelineSheetPath, TimelineSheetColumns, and
TimelineFrames. Unlike the burst, it is written even when nothing
changed and from a single tile, so an agent can see what a recording
showed without opening the video.
@hatayama
hatayama force-pushed the feature/record-video-timeline-sheet branch from 9881b0c to 69389f2 Compare October 21, 2026 06:41
The long-recording test kept passing when only the first thinning
doubled the interval, which leaves old stretches sparse and recent ones
dense. It now checks the final interval and that every gap between kept
tiles equals it.
@hatayama
hatayama merged commit ed5b6cc into feature/record-video-readable-evidence Oct 21, 2026
4 checks passed
@hatayama
hatayama deleted the feature/record-video-timeline-sheet branch October 21, 2026 06:57
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