Skip to content

fix(pronunciation): preserve dictionary ordering across backups - #2553

Closed
rudycelekli wants to merge 1 commit into
debpalash:mainfrom
rudycelekli:fix/pronunciation-backup-order-20261001
Closed

rudycelekli wants to merge 1 commit into
debpalash:mainfrom
rudycelekli:fix/pronunciation-backup-order-20261001

Conversation

@rudycelekli

@rudycelekli rudycelekli commented Oct 1, 2026 •

Copy link
Copy Markdown
Contributor

Summary

Fixes #2552.

Use created_at ASC, rowid ASC consistently 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

  • Use creation time and SQLite rowid tie ordering across pronunciation readers, with backup/restore regressions and relevant documentation.
  • Include relevant documentation and a credited changelog entry.

Type

  • 🐛 Bug fix
  • ✨ New feature
  • ♻️ Refactor
  • 📝 Documentation
  • 🧪 Tests
  • 🔧 CI / Build
  • 🚀 Release prep

Testing

  • Actual public FastAPI/SQLite import/export/replace-import: 8 of 20 case-variant pronunciations changed before; all 20 are preserved after.
  • Deterministic native regressions using identifier prefixes captured from the public import: 7 failed, 1 control passed before.
  • Final focused dictionary/Unicode/inert-entry/changelog suite: 83 passed, one existing TestClient deprecation warning; independently replayed 83 passed.
  • Exporter-only mutation restoring UUID tie ordering: 4 failed, 4 controls passed.
  • Controls cover creation-time precedence, exact and case-variant duplicates, repeated restores, global/scoped entries and disabled entries.

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 befc0a6f base; 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_at and SQLite rowid, 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

  • I've tested this locally (focused evidence and limitations above)
  • Every commit author has signed the CLA (current canonical CLA status and latest CLA gate pass; registered human signing verified, no agent signature submitted)
  • I've updated relevant documentation
  • No local machine paths, logs, or personal env details in this PR
  • Maintained version files are in sync (no owner-requested bump in this change)
  • Runtime regression fixture passes the full macOS/Windows/Linux smoke matrix (not claimed from focused local tests)

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 main
and 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.

Signed-off-by: Rudy Celekli <47457359+rudycelekli@users.noreply.github.com>
@greptile-apps

greptile-apps Bot commented Oct 1, 2026 •

Copy link
Copy Markdown
Contributor

Retrigger

[Medium risk] Fixes pronunciation dictionary ordering in backups and exports.

The PR appears safe to merge; no blocking issue was identified.

Summary

The 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 ..."

@coderabbitai

coderabbitai Bot commented Oct 1, 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: debpalash/VoiceStudio/.coderabbit.yaml

Review profile: CHILL

Plan: Advanced

Run ID: 3e7adecc-c398-446c-a587-513a657f7d9a

📥 Commits

Reviewing files that changed from the base of the PR and between befc0a6 and 68d6543.

📒 Files selected for processing (5)
  • CHANGELOG.md
  • backend/api/routers/pronunciation.py
  • backend/services/pronunciation.py
  • docs/expressive-speech.md
  • tests/test_pronunciation_backup_order.py

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


📝 Walkthrough

Walkthrough

Pronunciation 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.

Changes

Pronunciation ordering and backups

Layer / File(s) Summary
Consistent ordering across pronunciation queries
backend/api/routers/pronunciation.py, backend/services/pronunciation.py, docs/expressive-speech.md, CHANGELOG.md
Listing, dry-run, export, and synthesis loading order entries by creation time and then SQLite rowid. Documentation and the changelog describe the ordering and backup behavior.
Ordering and backup regression tests
tests/test_pronunciation_backup_order.py
Tests cover duplicate and case-variant precedence, creation-time ordering, and repeated backup round-trips for global, language-specific, and disabled entries.

Priority: ➖ Normal

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

Change: Bug fix · Severity of issue fixed: Medium

Suggested reviewers: debpalash

Merge Risk: ⚪ Minimal · up to 68d65

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)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning 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: … Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (8 passed)
Check name Status Explanation
Linked Issues check ✅ Passed Issue #2552 requires identical chronological ordering for list, dry run, synthesis loading, and export, with row insertion order as the tie-breaker. The reviewed code uses created_at ASC, rowid ASC …
Out of Scope Changes check ✅ Passed The changed router and service queries implement #2552. The added tests, documentation, and changelog entry support the ordering and backup behavior, and no unrelated implementation or public schema c…
Cross-Platform Default Parity ✅ Passed The PR changes default pronunciation ordering, but the behavior is platform-neutral. All four readers use standard SQLite ORDER BY created_at ASC, rowid ASC; the table is a normal rowid SQLite table…
I18n Completeness (21 Locales) ✅ Passed No frontend files are changed in the PR. The diff contains no new or changed frontend t('...') keys and no changed frontend user-facing strings that bypass i18n; therefore the 21-locale completeness c…
Local-First Guarantee ✅ Passed The PR changes only local SQLite ordering, documentation, and local FastAPI tests. The executable diff adds no network client, cloud call, account flow, API-key handling, telemetry, or reporting behav…
Backward Compatibility ✅ Passed The PR changes only pronunciation query ordering, documentation, changelog text, and tests. It does not modify omnivoice_data, backend/core/db.py, migrations, engine installation, or model-downloa…
Title check ✅ Passed The title uses the required conventional-commit format with the pronunciation scope and summarizes the backup-ordering fix. The description references issue #2552.
Description check ✅ Passed The description includes the required summary, changes, type, testing, checklist, and release-cadence sections. It reports testing evidence and clearly identifies the full platform matrix as not repro…
Full details: Docstring Coverage

Explanation

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.)

  • Fix all pre-merge checks with AI
  • Autopilot · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Autopilot is currently an internal CodeRabbit preview.


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.

@debpalash

Copy link
Copy Markdown
Owner

recheck

@github-actions

github-actions Bot commented Oct 3, 2026 •

Copy link
Copy Markdown
Contributor

All contributors on this pull request have signed the VoiceStudio CLA. Thank you!

@debpalash

Copy link
Copy Markdown
Owner

Thanks for the report and the work on this, @rudycelekli. The problem in #2552 is fixed on main by 95e6655b, so I'm closing this pull request as already fixed. It will ship in v0.5.7. Reporter credit stays in the changelog.

@debpalash debpalash closed this Oct 7, 2026
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.

Pronunciation dictionary backups change duplicate-entry precedence

2 participants