Repository navigation
Conversation
Intro Skipper now stores segments in introskipper-v2.db (table Segments, tick-based ranges, SegmentSource provenance and a State column) and leaves the legacy introskipper.db untouched after migrating. Prefer the v2 database, skip suppressed segments, treat SegmentSource.User as segment-editor entries, and fall back to the legacy database otherwise. Fixes intro-skipper#38 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
![kilo-code-bot[bot]](https://avatars.githubusercontent.com/in/2193792?s=60&v=4){kind=small}
Code Review SummaryStatus: 1 Issues Found | Recommendation: Address before merge Overview
Issue Details (click to expand)WARNING
Files Reviewed (2 files)
Fix these issues in Kilo Cloud Previous Review Summary (commit 62320ad)Current summary above is authoritative. Previous snapshots are kept for context only. Previous review (commit 62320ad)Status: 1 Issues Found | Recommendation: Address before merge Overview
Issue Details (click to expand)WARNING
Files Reviewed (2 files)
Reviewed by mimo-v2.6-pro · Input: 67.2K · Output: 22.7K · Cached: 562.4K |
Only use introskipper-v2.db when its Segments table has the columns the query relies on, so an empty, mid-migration or newer-schema file falls back to introskipper.db instead of failing the Share tab. Also restore skipme-index.js, which the local build regenerated and was committed by mistake. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
| { | ||
| // Fall back to the legacy database while the v2 file is missing, still being | ||
| // migrated, or from a schema revision this plugin does not understand. | ||
| if (File.Exists(_introSkipperDbPath) && HasV2SegmentsSchema(_introSkipperDbPath)) |
There was a problem hiding this comment.
WARNING: Schema probe throws instead of falling back when the v2 file is not a valid SQLite database
HasV2SegmentsSchema now rejects empty and schema-drifted v2 files as intended, but OpenReadOnly/ExecuteScalar still throw SqliteException ("file is not a database" / "database disk image is malformed") when introskipper-v2.db exists yet is corrupt or only partially written — precisely the mid-migration state the fallback comment above claims to cover. Nothing catches it, so ResolveIntroSkipperDbPath propagates the exception and GetShareableSegmentCountsByItemId (exposed via SkipMeDb/Share/Counts) returns 500 even when a fully populated legacy introskipper.db sits next to it. Treat a probe failure as "not v2" (wrap HasV2SegmentsSchema in a try/catch) so the legacy fallback below is always reached.
There was a problem hiding this comment.
An interrupted migration can't leave a half-written file, because SQLite rolls it back, and the schema check already covers the valid-but-incomplete case. A genuinely corrupt v2 db means Intro Skipper is broken too, and falling back would quietly upload stale timestamps from a file it stopped updating. I'd rather the error show up. The legacy path behaved the same way.
|
We're not currently accepting user submissions until there is a better way to verify them. |
Intro Skipper now stores segments in introskipper-v2.db (table Segments, tick-based ranges, SegmentSource provenance and a State column) and leaves the legacy introskipper.db untouched after migrating. Prefer the v2 database, skip suppressed segments, treat SegmentSource.User as segment-editor entries, and fall back to the legacy database otherwise.
Fixes #38
Note
Read Intro Skipper v2 segment database when sharing, with legacy fallback
ShareSubmissionService.cs now prefers the Intro Skipper v2 database for segment reads.
ResolveIntroSkipperDbPathpicks the v2 database when it exists and has the requiredSegmentsschema, falls back to the legacy database, and returns no path when neither is present.Sourcevalue. The legacy query path keeps dynamic detection of the olderIsUserProvidedandSourcecolumns.OpenReadOnlyhelper builds read-only SQLite connections for both paths.GetShareableSegmentCountsByItemIdnow uses the resolved path instead of only checking the v2 file's existence.Macroscope summarized c1cf272.