[claude] Standardize dry-run sync records and remove the Submit* defaults - #2514
[claude] Standardize dry-run sync records and remove the Submit* defaults#2514myieye wants to merge 4 commits into
Conversation
A dry run's only output is its records, so a record that can't be traced to an object is worthless. Many couldn't: creates were labelled by headword, gloss or name (none unique) with no id; AddSemanticDomainToSense and SetSensePartOfSpeech never recorded the sense at all; writing system records carried the type but not the WsId. Example sentence and picture records printed "MiniLcm.Models.RichString" because RichString had no ToString, so their content was silently empty. Only 5 of 17 update methods included the patch summary. Every record now carries the object's id, its parent when the method knows it, and the patch summary when there is one. Complex form component records name both entries and the link id: the component side alone doesn't identify a link, which made distinct records look like duplicates. The Submit* interface defaults are gone, which is what stops this recurring. They let a wrapper that must observe every write silently inherit one and record under the method it forwards to, naming an API the caller never used. They're now abstract, so each implementer states its own behaviour: FwData forwards (unchanged, moved out of the interface), and the recorder, WriteIgnoringMiniLcmApi and MiniLcmApiWriteNormalizationWrapper are compile-forced to be explicit. That last one is a behaviour fix. The normalization wrapper declared only SubmitUpdatePicture, so the other 11 fell through to its returning UpdateX and the CRDT re-read the object, throwing on one the other side had deleted instead of letting the delete win. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The sync-fidelity notes aren't on develop, so the pointer led nowhere. The reason it gave is short enough to state in place. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
backend/AGENTS.md asks for `return await` unless the method is a hot path; these run once per synced object, so the exemption doesn't apply. Covers the pre-existing SubmitUpdatePicture in the normalization wrapper too, so the block reads uniformly. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Dropped the convention essay on the recorder, the rationale duplicated between the interface and its implementers, and the notes that only described the change. Also drops a WriteIgnoring comment about inheriting an interface default, which no longer exists. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
Warning Review limit reached
Next review available in: 22 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (7)
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 |
github-actions
Bot
added
the
💻 FW Lite
|
The latest updates on your projects. Learn more about Argos notifications ↗︎
|
1 participant
[Claude, autonomous]
RecordingMiniLcmApinow writes standard descriptions that identify the entity: its id (headwords, glosses and names aren't unique), its parent, and the patch summary. Several records previously identified nothing.Submit*has no interface defaults any more, so every implementer writes them. The defaults cost compile-time checking in the classes that should implement all of them:MiniLcmApiWriteNormalizationWrapperhad silently inherited 11, degrading them to returning updates that re-read a possibly-deleted object instead of letting the delete win.RichString.ToString()returns its plain text. Rich values printed as their type name before, which left example sentence and picture records with no content.