Skip to content

Add method in recording service to return all ticks - #2449

Merged
ikondrat merged 8 commits into
mainfrom
kondrat-lig-10940-provide-information-about-current-ticks-of-mcap-sequence.service-get_ticks
Sep 21, 2026
Merged

ikondrat merged 8 commits into
mainfrom
kondrat-lig-10940-provide-information-about-current-ticks-of-mcap-sequence.service-get_ticks

Conversation

@ikondrat

@ikondrat ikondrat commented Sep 21, 2026

Copy link
Copy Markdown
Contributor

What has changed and why?

Add method in recording service to return all ticks

How has it been tested?

Nothing to test

Summary by CodeRabbit

  • New Features

    • Added access to ordered ticks for an MCAP sequence, including sequence numbers and timestamps.
    • Sequences without linked samples return an empty tick list.
    • Unknown sequences or sequences outside the specified dataset return no results.
  • Tests

    • Added coverage for ordered ticks, empty sequences, unknown sequences, and dataset mismatches.

@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Sep 21, 2026

Copy link
Copy Markdown

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review Status Commit Review trigger
📝 Code Review Completed 2026-09-21T15:33:54.292319Z d1c0e95 PR opened
ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

@coderabbitai

coderabbitai Bot commented Sep 21, 2026

Copy link
Copy Markdown
Contributor

Review Change StackReview Change Stack

Understand this PR’s impact

Explore downstream dependencies and potential security impact with Blast Radius.

View blast radius →

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Advanced

Run ID: bd0973f0-8ef3-48cc-a1ae-cd99b85dbfad

📥 Commits

Reviewing files that changed from the base of the PR and between d1c0e95 and 8e158d7.

📒 Files selected for processing (2)
  • lightly_studio/src/lightly_studio/services/recording_service/get_ticks.py
  • lightly_studio/tests/services/recording_service/test_get_ticks.py
🚧 Files skipped from review as they are similar to previous changes (1)
  • lightly_studio/tests/services/recording_service/test_get_ticks.py

Included review availability: Your plan provides up to 4 included reviews per hour; 2 remain after this review.


📝 Walkthrough

Walkthrough

The recording service now exposes get_ticks and validates sequence ownership through linked samples and collections. Tests cover ordered ticks, empty results, unknown sequences, and dataset mismatches.

Changes

Recording tick retrieval

Layer / File(s) Summary
Tick retrieval service and export
lightly_studio/src/lightly_studio/services/recording_service/get_ticks.py, lightly_studio/src/lightly_studio/services/recording_service/__init__.py
Checks dataset ownership through sequence, sample, and collection resolvers. Retrieves sample links and exports get_ticks.
Tick retrieval validation
lightly_studio/tests/services/recording_service/test_get_ticks.py
Tests ordered tick values, empty tick lists, unknown sequences, and mismatched dataset IDs.

Priority: ⬇️ Low

Estimated code review effort: 2 (Simple) | ~10 minutes

Change: Feature

Sequence Diagram(s)

sequenceDiagram
  participant get_ticks
  participant mcap_group_sequence_resolver
  participant sample_resolver
  participant collection_resolver
  participant sequence_resolver
  get_ticks->>mcap_group_sequence_resolver: Check sequence existence
  get_ticks->>sample_resolver: Resolve linked sample
  get_ticks->>collection_resolver: Resolve sample collection
  get_ticks->>sequence_resolver: Get sample links
  sequence_resolver-->>get_ticks: Return tick data
Loading
🚥 Pre-merge checks | ✅ 3 | ❌ 2

❌ Failed checks (2 warnings)

Check name Status Explanation Resolution
Description check ⚠️ Warning The description identifies the main change but omits the required changelog section and gives inaccurate testing information. The pull request adds tests, so “Nothing to test” is incomplete. Add the changelog checklist and describe the tests in test_get_ticks.py, including the covered cases and how to run them. Include the motivation, issue context, and any dependencies if applicable.
Docstring Coverage ⚠️ Warning Docstring coverage is 33.33% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 6 functions across 3 files. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (3 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly describes the main change: adding a recording service method that returns all ticks.
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.
  • Fix all pre-merge checks with AI
✨ 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

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.

@ikondrat
ikondrat enabled auto-merge September 21, 2026 15:30

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: d1c0e95752

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread lightly_studio/src/lightly_studio/services/recording_service/get_ticks.py Outdated
Comment thread lightly_studio/src/lightly_studio/services/recording_service/get_ticks.py Outdated
Comment thread lightly_studio/src/lightly_studio/services/recording_service/get_ticks.py Outdated
Comment thread lightly_studio/tests/services/recording_service/test_get_ticks.py Outdated
Comment thread lightly_studio/tests/services/recording_service/test_get_ticks.py Outdated
@lightly-fast-track-bot

lightly-fast-track-bot Bot commented Sep 21, 2026

Copy link
Copy Markdown

✅  Fast Track: all required checks passed — auto-approved.

Guardrail Result Message
frontend/complexity 0 file(s) checked.
backend/complexity 3 file(s) checked, no violations.
backend/coverage 1 file(s) checked at 90%.
[PASS] lightly_studio/src/lightly_studio/services/recording_service/get_ticks.py: 94.7%
diff-size PR adds 155 line(s) (limit: 215).
frontend/coverage 0 file(s) checked.

View the guardrail run

To run the guardrails locally, from fast_track/ run make install once, then make run-guardrails (or GUARDRAILS=<name1>,<name2> make run-guardrails for some guardrails).

Reflects 8e158d7.

@ikondrat
ikondrat force-pushed the kondrat-lig-10940-provide-information-about-current-ticks-of-mcap-sequence.service-get_ticks branch from 8909fc6 to d1c0e95 Compare September 21, 2026 15:38
- Derive sequence-dataset membership from the sequence sample's own
  collection instead of its recording, and validate via get_by_id so a
  sequence created before its GROUP schema is not reported as missing.
- Drop the per-tick channel locator claim from the module docstring.
- Import uuid and helpers modules in tests per import conventions.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@ikondrat
ikondrat added this pull request to the merge queue Sep 21, 2026
Merged via the queue into main with commit 8898c49 Sep 21, 2026
28 checks passed
@ikondrat
ikondrat deleted the kondrat-lig-10940-provide-information-about-current-ticks-of-mcap-sequence.service-get_ticks branch September 21, 2026 16:07
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