Conversation
LF stores a writing system tag as it was typed, but LCM canonicalizes a tag when it creates the writing system and thereafter knows it only by that canonical id. TryGet and GetWsFromStr both match the id literally (they are case-insensitive, but not structure-aware), so a tag whose canonical form differs structurally is never found: qaa-x-qaa-v -> qaa-x-v (redundant private-use "qaa" absorbed) th-Thai -> th (suppress-script dropped) LfWsToLcmWs then takes its else branch and calls Create() -- which DOES canonicalize -- so Set() collides with the writing system already there: Unable to set writing system 'qaa-x-kal' because this id already exists. The second test covers the quieter half: LfMultiText resolves each multitext key with GetWsFromStr and skips anything returning 0, so a value keyed by a non-canonical tag is dropped, and the clearing pass then blanks whatever LCM had for that field. testlangproj already contains qaa-x-kal and qaa-Zxxx-x-kal-audio, so these use a non-canonical spelling of an existing writing system rather than adding to the shared fixture project. The qaa-Zxxx-x-kal-AUDIO case already passes (case-only difference) and is a guard against regressing it. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
LF stores a writing system tag exactly as it was typed; LCM canonicalizes it when the writing system is created and thereafter knows it only by that canonical id. GetWsFromStr matches that id literally -- case-insensitively, but not structure-aware -- so a multitext keyed by a tag whose canonical form differs structurally resolves to 0: qaa-x-qaa-v -> qaa-x-v (redundant private-use "qaa" absorbed) th-Thai -> th (suppress-script dropped) WriteToLcmMultiString skips a key that resolves to 0, and its clearing pass then blanks whatever LCM held for that writing system, so the value is not merely dropped on the way in, it is deleted on the way out. Canonicalize the key before the lookup, here and in WsIdAndFirstNonEmptyString. LanguageTags.Canonical guards IetfLanguageTag.Canonicalize with IsValid, which it needs: Canonicalize throws on a tag it cannot parse, and a corpus scan found 48 such strings (custom field names that look like multitext keys). Verified against SIL.WritingSystems: Canonicalize agrees with the id liblcm itself stores for all 1251 valid tags in that corpus, and is idempotent on all of them. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
TryGet matched the writing system id literally while Create canonicalized it, so a tag whose canonical form differs -- "qaa-x-qaa-v" becomes "qaa-x-v", because the private-use section repeats the "qaa" language subtag -- was not found, was created under its canonical id, and collided with the writing system already there: Unable to set writing system 'qaa-x-v' because this id already exists. GetOrSet does the lookup and the creation in one step and canonicalizes for both, so the two halves can no longer disagree. It returns true when it found an existing writing system and false when it created one, and a created one is already in the manager, so the separate Set call goes away and the two branches collapse into one path plus a conditional vernacular/analysis assignment. This implements only half of the TODO it replaces. The other half -- "use the ILgWritingSystemFactory interface and remove one point of FW 8-to-9 API incompatibility" -- does not follow: GetOrSet is declared on WritingSystemManager, not on ILgWritingSystemFactory, which offers only get_Engine, GetWsFromStr, GetStrFromWs, GetIcuLocaleFromWs and GetWritingSystems. The #if FW8_COMPAT blocks therefore stay, and the comment now records why rather than leaving a TODO that cannot be done as written. The vernacular comparison keeps comparing the tag as LF spells it against languageCode as LF spells it. Both are LF's own strings, so a project whose languageCode is itself non-canonical still matches its own input system. Braces added to the single-statement Abbreviation guard, which reads as indentation-scoped without them. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
LF has no vernacular/analysis flag on a writing system; the distinction is conventional, encoded in which config fields use it. LfWsToLcmWs treated exactly one writing system as vernacular -- whichever equalled the project's languageCode -- and filed every other one as analysis. A project whose lexeme field uses more than one writing system therefore lost the distinction: apltw2016's lexeme field uses qaa-x-IPA-dupl1 alongside th, and the phonetic one was made analysis. Take the vernacular writing systems from the fields that are vernacular by convention instead: citationForm, lexeme, etymology, senses.fields.examples.fields.sentence That list was already in MongoConnection, which applies the same convention in the other direction when it writes input systems back out to the config. Rather than have a second copy to keep in step by hand, it moves to MagicStrings.LfVernacularConfigFieldPaths and both directions read it from there. MongoConnection keeps its field name, so its six use sites are unchanged; the type widens to IReadOnlyList so the shared instance is not handed out as mutable. The example sentence path is nested, so the paths are walked a segment at a time. They are spelled as Mongo sees them, where each level of nesting goes through a "fields" document; in the mapped classes that is the Fields dictionary being indexed, so those segments are skipped. Where the config names no vernacular input systems at all, the project's language code is still used, so a project with a config LfMerge cannot read behaves as it did before. Tags are compared case-insensitively, matching LCM's own writing system lookups. This replaces the "needs to be a bit fuzzier" TODO in that method. Note the assignment still only happens for a writing system LfMerge has just created; one that already exists in the FieldWorks project keeps whatever vernacular/analysis membership it has there. That is unchanged behaviour, and it is why the new tests cover the derivation directly: creating a writing system in the shared test project cannot be undone afterwards, since WritingSystemManager has no removal API. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Some older projects had a missing or null DirtySR field, which was causing Mongo to throw an error because we never told it to treat null values as 0 for that int field. It's a simple one-line fix to treat missing values as 0, but treating null values as 0 requires a custom serialization helper. Still simple enough, thankfully.
The previous commit took the vernacular writing systems from a fixed list of config fields -- citationForm, lexeme, etymology and the example sentence -- shared with MongoConnection. Checking that against the 2026-07-06 corpus showed one list cannot be right for every project: 741 projects configure etymology with analysis writing systems and only 378 with vernacular ones, so naming etymology vernacular adds "en" to the vernacular list of hundreds of projects that have no English headwords. Classify per project from its own config instead. The lexeme and citation form fields are vernacular by definition and the sense definition and gloss are analysis by definition, so they anchor the decision; every other field carrying input systems -- etymology, the example sentence, custom fields -- is assigned by which anchor its writing systems already appear in. Only a writing system exclusive to one anchor counts as evidence: "en" is in both in many projects and so says nothing about any other field's role. This keeps what the fixed list already got right over the original languageCode rule. flh-flex has languageCode "flh-x-ortho", a writing system only its etymology field uses, so its real lexeme writing system "flh-x-cm" was classified as analysis; khg-yl-flex has the bare tag "khg" while its lexeme uses "khg-CN-x-Yangla" and "khg-fonipa"; odo has languageCode "th", which is not among its writing systems at all, leaving its 24,460 "en" headwords as analysis and the project with no vernacular writing system whatsoever. All three still come out right. Writing systems that overlap neither anchor are treated as vernacular and reported. 11 projects need this; grc-vie-flex is the clearest, a Koine Greek dictionary glossed in English and Vietnamese whose etymologies are in Hebrew and Aramaic -- neither vernacular nor analysis. The FieldWorks fields those feed are vernacular-typed, so calling them analysis would leave the data unreachable, whereas a spare vernacular writing system is inert. Since the two directions no longer share a rule, MongoConnection gets its own list back and MagicStrings.LfVernacularConfigFieldPaths goes away. MongoConnection answers a different question -- which config fields to write the vernacular writing systems into -- and that divergence is now deliberate rather than drift. languageCode survives as a last-resort fallback, for a config LfMerge cannot read at all: without it such a project would end up with no vernacular writing system, which is worse than the rule this replaces. Note this only affects writing systems LfWsToLcmWs creates. A project already synced under an earlier rule keeps the membership its FieldWorks project has. The tests move with the logic, keeping their coverage of the union, case-insensitive matching, the nested example sentence path, apltw2016's second lexeme writing system and the fallbacks. Two of them had described configs that do not match how Mongo spells them, with definition and gloss at the entry level rather than under senses; the anchor paths are strict about that, so those configs are corrected. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
SetCustomFieldData had no case for CellarPropertyType.MultiUnicode, so every multilingual text custom field fell through to `default: return false` and its data was dropped. The other direction has handled MultiUnicode all along (ConvertLcmToMongoCustomField line 363), so LfMerge could read one of these fields out of FieldWorks but never write one back in. That asymmetry is invisible in a log. The name lookup succeeds -- the field exists in LCM, so it is not among the remainingFieldNames that produce "Custom field X from LF skipped" -- and the default case returned false without saying anything, carrying a TODO asking for exactly the warning it never got. Migrating thai_jay is what turned it up: its "Top 10 list" field arrived in FieldWorks present but empty for all 537 entries, with nothing in 10,801 lines of LfMerge output to explain why. The comment above the switch is the likely origin, since it lists the types to implement and leaves MultiUnicode out; it is wrong, and is corrected here. This matters more than a missing type usually would, because MultiUnicode is the type every text custom field in a Language Forge project has. LF has never offered a single-string custom field, so nothing LF owns arrives as CellarPropertyType.String -- the one text type that did work. Alternatives LF does not have are cleared rather than left in place. That is safe here specifically because the export writes every alternative LCM holds, via LfMultiText.FromMultiITsString: an alternative missing from LF is one a user deleted there, not one LF never saw. It also matches what LfMultiText.WriteToLcmMultiString does for the built-in multitext fields. Values that have not changed are left untouched, as the String case does, so they stay out of the .fwdata XML and out of the Mercurial commit. MultiString is deliberately not added. GetCustomFieldData cannot produce one either, so an import path for it would be untested code for a shape LfMerge never emits; it now reaches the new warning instead of disappearing. The default case logs a warning, which is what the TODO asked for and what the LCM-to-Mongo direction has always done. A field type going unimplemented is worth a line in the log rather than silence. Tests cover the four cases against the test project's Cust_Single_Line_All, which really is MultiUnicode with English and French alternatives: both alternatives written, a previously emptied field written (the migration case), an alternative LF dropped cleared rather than left stale, and an unidentified writing system skipped. All four fail on the unmodified converter, each showing the field still holding its original "Some custom text". Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude determined that this warning is no longer needed; there are indeed some LF fields (usually on the sense level) that have empty but non-null multitexts, but they truly are empty and there is no loss of data involved, so no need for a spammy log warning.
Collaborator
Author
|
Only one project in production could end up with data issues after this PR is merged, because it did the equivalent of having both |
Collaborator
Author
|
Currently having Claude do code review. Will post results when completed. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Some projects that were not yet in the Send/Receive system were using writing system tags that did not canonicalize correctly, which was leading to FLEx seeing a different writing system than LfMerge. This is because LfMerge was storing writing systems using the ID that users gave (e.g.,
qaa-x-qaa-vbut liblcm (and FLEx) canonicalize the writing system ID before storing it (e.g.,qaa-x-v). This was resulting in LfMerge appearing to lose data, as allqaa-x-vfields were showing up empty because LfMerge was (incorrectly) treating those as different from theqaa-x-qaa-vwriting system.This PR fixes the issue by having LfMerge canonicalize writing systems before storing them, the same way liblcm does. I have also included a couple other minor bugfixes, like dealing with a missing
DirtySRvalue in the Mongo database: all Send/Receive projects had the correct DirtySR value, but a few non-SR projects that hadn't been touched in a while had a missingDirtySRvalue, which caused LfMerge to throw a Mongo serialization error instead of correctly treating it as 0. That was a minor fix so I folded it into this same PR for efficiency.