Repository navigation
fix(pronunciation): preserve dictionary ordering across backups - #2553
rudycelekli wants to merge 1 commit into
Conversation
Signed-off-by: Rudy Celekli <47457359+rudycelekli@users.noreply.github.com>
|
[Medium risk] Fixes pronunciation dictionary ordering in backups and exports. The PR appears safe to merge; no blocking issue was identified. SummaryThe PR uses creation time and insertion order across dictionary readers, then adds backup-order regression tests and updates the documentation and changelog. No merge-changing issue was identified. Reviews (1) · Last reviewed commit: "fix(pronunciation): preserve dictionary ..." |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository: debpalash/VoiceStudio/.coderabbit.yaml Review profile: CHILL Plan: Advanced Run ID: 📒 Files selected for processing (5)
Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 2 remain after this review. 📝 WalkthroughWalkthroughPronunciation listing, preview, export, and synthesis loading now order entries by creation time, using SQLite insertion order to break timestamp ties. Regression tests cover duplicate and case-variant precedence, creation-time ordering, and repeated backup round-trips. ChangesPronunciation ordering and backups
Priority: ➖ Normal Estimated code review effort: 2 (Simple) | ~10 minutes Change: Bug fix · Severity of issue fixed: Medium Suggested reviewers: Merge Risk: ⚪ Minimal · up to This change makes pronunciation dictionary ordering consistent across listing, preview, export and synthesis, so backup and restore preserve duplicate-entry precedence. No concrete merge-blocking risk was found. 🚥 Pre-merge checks | ✅ 8 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (8 passed)
Full details: Docstring CoverageExplanation Docstring coverage is 27.27% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 11 functions across 3 files. (2 skipped: 2 unsupported.)
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. Comment |
|
recheck |
|
All contributors on this pull request have signed the VoiceStudio CLA. Thank you! |
|
Thanks for the report and the work on this, @rudycelekli. The problem in #2552 is fixed on |
Summary
Fixes #2552.
Use
created_at ASC, rowid ASCconsistently for pronunciation dictionary lists, dry runs, exports and the synthesis entry loader. Creation-time ordering is preserved; entries from the same bulk import retain their authored order instead of sorting by random UUIDs. Repeated export/import preserves duplicate and case-variant precedence.Four SQL reader ordering changes; no public schema, migration or dependency changes. Docs and a credited changelog entry are included.
Changes
Type
Testing
No provider/model calls. The actual synthesis dictionary loader is exercised directly; no inference-accuracy or full model-generation claim. The full ML/Bun/platform matrix has not been reproduced in the reduced local environment and remains upstream CI.
Scope checks
Fresh
befc0a6fbase; complete PR metadata, 22 relevant historical actual file packets and all 20 current open PR packets screened. #2550 shares the dictionary files but changes language normalization only; its complete target patches contain no SQL-order hunks. This PR repairs ordering independently. Signed commits include DCO sign-off.The dictionary list, dry run, export, and synthesis loader now order entries by
created_atand SQLiterowid, preserving insertion order for timestamp ties; regression tests cover precedence through repeated backup and restore. This prevents duplicate and case-variant pronunciations from changing after restore. The docs and changelog also attribute tie-ordering to previews, but the preview endpoint was not changed, so verify that claim before merging.Checklist
Release cadence
VoiceStudio ships continuous-to-main — no release candidates, no soak windows.
Every merged PR is immediately part of rolling source (
main) and Docker:latest. Electron artifact rehearsals validate desktop packages without publishing.Version bumps require owner approval; validated releases are tagged from
mainand published explicitly under the release checklist.
Users who want stability install an Electron release or pin Docker
:stable.Current aggregation context
Owner PR #2585 explicitly lists this branch among its superseded contributions. That is an aggregation statement, not permission to assume every compatibility or hosted gate is resolved. The individual branch has not been blindly rebased or merged into the owner branch.