Skip to content

fix(cli): make 'lingo.dev lockfile' additively populate missing sections#2093

Open
cherkanovart wants to merge 1 commit into
mainfrom
fix/lockfile-merge-missing-sections
Open

fix(cli): make 'lingo.dev lockfile' additively populate missing sections#2093
cherkanovart wants to merge 1 commit into
mainfrom
fix/lockfile-merge-missing-sections

Conversation

@cherkanovart
Copy link
Copy Markdown
Contributor

@cherkanovart cherkanovart commented May 20, 2026

Summary

Builds on #2091. PR #2091 patched the --frozen false-positive only for the narrow case where a no-op lingo.dev run precedes --frozen. This PR closes the remaining cases by changing lingo.dev lockfile from a bail-out into the actual recovery command, and by updating --frozen errors to point users at it.

Failure scenarios this closes (that #2091 left open)

  • New file appears under a ** recursive glob → new pathPattern → no lock section.
  • New bucket added to i18n.json.
  • Previous run used --target-locale (execute.ts gates saveChecksums on !flags.targetLocale?.length).
  • Existing i18n.lock with checksums: {} (or any section that exists but is empty).

All four funnel through the same throw at frozen.ts:127–131 because loadChecksums returns {} for a missing section and every key in src compares unequal to undefined.

Behavior change: lingo.dev lockfile

Without --force:

  • Iterate getBuckets(i18nConfig).
  • For each bucketConfig.pathPattern, check MD5(pathPattern) against the lock's checksums map.
  • If the section exists with at least one checksum → skip (preserve the divergence signal that --frozen relies on).
  • Otherwise → pull source data, call registerSourceData (full-section write).

--force retains the previous full-rebuild semantics.

Per-path stdout: added, skipped (already populated), replaced (--force), plus a summary line.

A new public method hasSourceData(pathPattern: string): boolean was added to createLockfileHelper() to expose the section-presence check cleanly.

registerPartialSourceData is deliberately not used — its _.merge would overwrite individual checksums in a non-empty section and re-introduce the masking risk (a source value flip "Sasha""Petr" would silently get re-baselined). The whole point of this design is that any non-empty section is load-bearing and only --force may touch it.

Error-message update: frozen.ts

All four throws (lines 129/147/157/167) now read:

Localization data has changed. Run `lingo.dev lockfile` to refresh i18n.lock, or run without --frozen. Details: …

The existing Details: … suffixes are unchanged.

Why not auto-heal inside --frozen?

I considered self-healing inside frozen.ts (initialize missing sections automatically). Rejected because:

  1. It shifts the meaning of --frozen from "strict no-drift validation" to "validate, but auto-fill missing entries" — out of line with npm ci, yarn --immutable, cargo --locked, bundler --frozen.
  2. There's a one-shot masking risk: if a source value is flipped at the same moment the lock section is missing, validation silently passes with stale target translations.

The explicit "run a command, commit the lock" flow has no analogous failure mode and matches user expectations for a lockfile-based tool.

Test plan

  • New: src/cli/utils/lockfile.test.ts — 4 hasSourceData tests (absent lock, absent section, empty section, populated section).
  • New: src/cli/cmd/lockfile.spec.ts — 5 integration scenarios:
    • Partial fill: lock has section A, missing section B → both populated after run, A byte-identical.
    • Empty lock (checksums: {}) → section created.
    • Stale checksum preserved: lock has stale entry for P, source modified → entry untouched (so --frozen still throws the divergence signal).
    • --force regression guard: existing section fully rebuilt.
    • ** recursive glob: 2 files run, add 3rd, run again — only the 3rd section is added; first two byte-identical.
  • pnpm test in packages/cli/: Test Files 55 passed (55) | Tests 886 passed (886).

Follow-ups (intentionally not in this PR)

  1. delta.ts:104–112 and lockfile.ts:72–78 rewrite the lock during a read when deduplicateLockfileYaml finds duplicates — so --frozen isn't actually read-only today. Worth a separate PR to skip the rewrite from --frozen, and to harden the YAML round-trip (parseDocument → toJSON → new Document → toString) against translation keys that look like YAML specials (yes/no, ISO dates, numbers).
  2. Opt-in lingo.dev run --frozen --heal to inline the merge step before validation. Hold until there's user demand; the two-command flow is the default we want.

Summary by CodeRabbit

Release Notes

  • New Features

    • Lockfile command now fills missing sections additively instead of aborting; --force still rebuilds entirely.
  • Bug Fixes

    • Resolved false-positive --frozen validation failures with wildcard path patterns and empty sections.
    • Improved --frozen error messages to guide users to the recovery command.
  • Tests

    • Added comprehensive test coverage for lockfile merge and update behaviors.

Review Change Stack

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented May 20, 2026

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: fe534a94-6413-4845-bd46-f3eda331adfd

📥 Commits

Reviewing files that changed from the base of the PR and between 0f26278 and 2e08ab8.

📒 Files selected for processing (6)
  • .changeset/lockfile-merge-missing-sections.md
  • packages/cli/src/cli/cmd/lockfile.spec.ts
  • packages/cli/src/cli/cmd/lockfile.ts
  • packages/cli/src/cli/cmd/run/frozen.ts
  • packages/cli/src/cli/utils/lockfile.test.ts
  • packages/cli/src/cli/utils/lockfile.ts

📝 Walkthrough

Walkthrough

The PR implements additive merge behavior for lingo.dev lockfile, allowing the command to populate missing i18n.lock sections without overwriting existing checksums unless --force is used. It adds a hasSourceData() helper to detect pre-existing sections, refactors the command loop to skip populated sections, provides comprehensive test coverage, and updates user-facing error guidance.

Changes

Lockfile Additive Merge Behavior

Layer / File(s) Summary
Feature documentation
.changeset/lockfile-merge-missing-sections.md
Changeset entry documents the new additive merge semantics, section preservation without --force, and updated --frozen error direction.
Lockfile helper: hasSourceData detection
packages/cli/src/cli/utils/lockfile.ts, packages/cli/src/cli/utils/lockfile.test.ts
New hasSourceData(pathPattern) method queries the lockfile checksum section to determine if source data has been previously captured; test suite validates behavior across missing/empty/populated sections.
Lockfile command: merge implementation
packages/cli/src/cli/cmd/lockfile.ts
Command action refactored to compute lockfile existence, track added/skipped/replaced counters, skip bucket sections when hasSourceData() returns true and --force is not set, and emit contextual success messages.
Lockfile merge test suite
packages/cli/src/cli/cmd/lockfile.spec.ts
Comprehensive test environment with real disk I/O, helper functions, and five test cases: missing sections fill additively, empty checksums populate, source divergence is preserved, --force rebuilds sections, and recursive globs add entries incrementally.
Frozen validation error guidance
packages/cli/src/cli/cmd/run/frozen.ts
Four error messages in source/target mismatch validation paths updated to direct users to run lingo.dev lockfile for lock file refresh.

🎯 3 (Moderate) | ⏱️ ~25 minutes

Possibly related PRs

  • lingodotdev/lingo.dev#2004: Refactors createBucketLoader() initialization to accept an options object; this PR depends on and uses that same initialization pattern.

Suggested reviewers

  • vrcprl
  • AndreyHirsa

🐰 A lockfile once was scattered and forlorn,
Missing sections causing --frozen scorn,
But now we merge the pieces back together whole,
With hasSourceData wisdom in each role,
No more rebuilds for every little whim—
Just fill the gaps and keep the checksums trim! 📋✨

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly and accurately summarizes the main change: making 'lingo.dev lockfile' additively populate missing sections rather than aborting.
Description check ✅ Passed The description covers all required sections: a summary, detailed explanation of changes and test plan. The changeset file is included, and tests are documented as passing (886 passed).
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.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/lockfile-merge-missing-sections

Comment @coderabbitai help to get the list of available commands and usage tips.

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.

2 participants