fix(member-fields): reconcile rows that carry no referenceName - #160
Merged
Conversation
Review follow-up to #159. Making `referenceName` the exact identity left the documented name fallback unreachable and pointed several diagnostics at a remedy that did not work. - A live row with no `referenceName` was matched by name and then rejected by every consumer, so it could be neither updated nor created: the plan went INCOMPLETE and apply aborted the whole run. Only a row carrying a DIFFERENT reference name is a contradiction now (`conflictingReferenceName`). - The actual side reports the declared reference name for such a row, since ct never PATCHes one onto it and diffing it would never converge. - A declared field whose live row has no reference name is no longer reported as a DELETE CANDIDATE; the pass now skips rows a declaration claimed. - Identity-mismatch messages offer the non-destructive fix (declare the live `referenceName`) beside `ct destroy`, and `ct destroy --member-field` resolves the STATE-BOUND row — it previously matched on the local key alone, reported "already absent", dropped the binding and let the next apply POST a duplicate. - A live field that only shares a declaration's display name now warns and creates instead of failing the run; a near-identity (same string up to punctuation/case) is still refused. - A ref into a group that is adopted but declares no `memberFields` states no exact identity, so it keeps matching on the normalised local key. - The duplicate-match error no longer claims no row carries the exact reference name when the matches are the rows that do. Claude-Session: https://claude.ai/code/session_01PNmsG3pmvxQNA9sNmnAvYJ
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
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.
Review follow-up to #159 (7 findings from
/code-review high).High
referenceNamewas matched by the documented name fallback and then rejected by every consumer (memberFieldReferenceName(row) !== spec.referenceName), so it could be neither updated nor created — the plan went INCOMPLETE andct applyaborted the entire run. Only a row carrying a different reference name is a contradiction now (conflictingReferenceName), in the fold, in apply and in the resolver.ct destroy --member-field …, which is destructive and, for a row without a reference name, did not even match:destroyresolved the live row by local key alone, reported "already absent", dropped the state binding and let the next apply POST a duplicate.destroy(and its backup fetch) now prefer the state-bound id, and every message names the non-destructive fix — declare the livereferenceName— beside the destructive one.Medium
?? "", so a declared field whose live row has none was reported as "not declared — DELETE CANDIDATE". The pass now skips rows a declaration claimed.memberFieldsfell back to the raw ref string with exact matching, hard-throwing on livestand-bewerbungvs.ref.groupMemberField(g, "stand_bewerbung"). With no declaration there is no exact identity to hold the row to, so those refs keep the normalised local-key match.referenceName) turned a create into a fold error and thereby aborted apply for every other resource. It now warns, names thereferenceNameto declare, and plans the create. A near-identity — a live reference name differing only in punctuation or case — is still refused.Low
Also: the actual side reports the declared reference name for a row that carries none — ct never PATCHes one, so diffing it would propose the same update forever.
Docs re-read and re-signed.
npm test(1083 passing),tsc --noEmit,eslint src testsall clean; 9 regression tests added.https://claude.ai/code/session_01PNmsG3pmvxQNA9sNmnAvYJ