feat(core): add ordered flag to attribute schema to ignore list eleme…#9782
feat(core): add ordered flag to attribute schema to ignore list eleme…#9782ajtmccarty wants to merge 6 commits into
Conversation
There was a problem hiding this comment.
1 issue found across 10 files
Confidence score: 4/5
- In
changelog/9764.fixed.md, the documented default fororderedconflicts with the schema (falsevs actualtrue), which can mislead users into relying on the wrong behavior and cause unexpected conflict handling after upgrade; update the changelog text (including the “default” parenthetical) to match the real default before merging.
Shadow auto-approve: would not auto-approve because issues were found.
Re-trigger cubic
There was a problem hiding this comment.
0 issues found across 2 files (changes from recent commits).
Shadow auto-approve: would require human review. Adds new ordered flag to attribute schemas and modifies core conflict detection logic for diff/merge. This is a feature change in a critical path that requires human review to ensure correctness and no unintended side effects.
Re-trigger cubic
There was a problem hiding this comment.
The List path is well covered, but the "JSON" branch of _UNORDERED_CAPABLE_KINDS never fires in any test. Could we add one JSON attribute with an array value (proves the JSON branch works) and one where the JSON value is a top-level dict (locks in the exact-comparison fallback)?
…nt order in diffs Adds an `ordered` boolean to AttributeSchema (default True). When set to False on a List or JSON-array attribute, reordering its elements is no longer reported as a conflict during branch merge or rebase; element counts still matter, so adding or removing an element remains a conflict. The diff conflict enricher compares such attributes as multisets and resolves the schema from the diff branch first, so a branch's own `ordered` change is honored within its diff. The built-in schema `enum`, dropdown `choices`, generic `used_by`, and `restricted_namespaces` attributes are marked ordered=False. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Regenerate frontend/app/src/shared/api/rest/types.generated.ts after adding the `ordered` field to schema/openapi.json, fixing the frontend-validate-openapi-types CI check. Drop the incorrect "(the default)" qualifier in the changelog: the `ordered` attribute-schema field defaults to `true` (order-sensitive), not `false`. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Regenerating the frontend OpenAPI REST types for the new `ordered` field changed the type structures that TypeScript embeds in several existing tracked error messages. The error count and locations are unchanged (212 issues); only the message text differs. Refresh .betterer.results so `betterer ci` passes. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
361f3fb to
3f78c61
Compare
There was a problem hiding this comment.
1 issue found across 3 files (changes from recent commits).
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="python_sdk">
<violation number="1" location="python_sdk:1">
P2: The SDK submodule bump (e0c27a14) adds MERGE_FAILED to BranchStatus but does not add the new `ordered` attribute schema field that this PR introduces. AttributeSchema in the SDK is a Pydantic model that deserializes server schema responses — without `ordered`, SDK-based clients that fetch attribute schemas (e.g., infrahubctl schema dump) will silently drop the field. Consider adding `ordered: bool = True` to AttributeSchema so the SDK round-trips the new field correctly.</violation>
</file>
Shadow auto-approve: would not auto-approve because issues were found.
Tip: Review your code locally with the cubic CLI to iterate faster.
Re-trigger cubic
| @@ -1 +1 @@ | |||
| Subproject commit b76594a5ce659265832cad22f4406e46af21d3cb | |||
| Subproject commit e0c27a149d8d5442805d5660c0d86869a30d3298 | |||
There was a problem hiding this comment.
P2: The SDK submodule bump (e0c27a14) adds MERGE_FAILED to BranchStatus but does not add the new ordered attribute schema field that this PR introduces. AttributeSchema in the SDK is a Pydantic model that deserializes server schema responses — without ordered, SDK-based clients that fetch attribute schemas (e.g., infrahubctl schema dump) will silently drop the field. Consider adding ordered: bool = True to AttributeSchema so the SDK round-trips the new field correctly.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At python_sdk, line 1:
<comment>The SDK submodule bump (e0c27a14) adds MERGE_FAILED to BranchStatus but does not add the new `ordered` attribute schema field that this PR introduces. AttributeSchema in the SDK is a Pydantic model that deserializes server schema responses — without `ordered`, SDK-based clients that fetch attribute schemas (e.g., infrahubctl schema dump) will silently drop the field. Consider adding `ordered: bool = True` to AttributeSchema so the SDK round-trips the new field correctly.</comment>
<file context>
@@ -1 +1 @@
-Subproject commit b76594a5ce659265832cad22f4406e46af21d3cb
+Subproject commit e0c27a149d8d5442805d5660c0d86869a30d3298
</file context>
Why
When two branches both edit the same list-valued attribute (a
List/JSONattribute, or aDropdown'schoices, an attribute'senum, etc.) so that they end up holding the same elementsin a different order, the diff reports a value conflict and merge/rebase is refused — even though
there is no semantic divergence. This is the converged-state dead-end described in #9764 for
Dropdown.choices: once both branches hold the same set of choices, the branch still cannot rebase.Goal: let a schema author declare that element order is not significant for an attribute, so
reordering (or converging to the same set in a different order) is not treated as a conflict.
Non-goals: this does not change the rebase guard to honor
ResolveDiffConflictselections— that is a separate change. It also does not alter merge value resolution: when there is no
conflict, the merge continues to apply the branch's value as before.
Closes #9764 (resolves the converged-choices dead-end; the rebase-guard resolution handling is
out of scope here and is captured in #9654).
What changed
Behavioral changes
orderedboolean on attribute schemas (defaultTrue). When set toFalseon aListorJSON-array attribute, reordering its elements is no longer a conflict during branch merge orrebase. Element multiplicity still matters — adding or removing an element is still a conflict.
enum, dropdownchoices, genericused_by, andrestricted_namespacesare markedordered=False, so two branches converging to the same set ofvalues in a different order no longer conflict.
Implementation notes
ConflictsEnricher._have_same_value. For an unorderedattribute it now compares the two JSON values as multisets (canonicalizing each element), falling
back to exact comparison when a value is not a JSON array.
changes an attribute's
orderedflag has that change honored within its own diff. Schema lookupsdegrade gracefully (fall back to order-sensitive) when a schema/branch cannot be resolved.
orderedusesUpdateSupport.ALLOWED: it only affects how diffs are interpreted, never storeddata, so no data migration is required (unlike
unique/read_only).What stayed the same
ordered=True): order-sensitive conflict detection for everyattribute that does not opt out.
update_branch_diffandrecalculatepaths) — a conflict recorded during a real divergence iscleared once the two sides converge.
Suggested review order
backend/infrahub/core/diff/conflicts_enricher.py— the core comparison and schema resolution.backend/infrahub/core/schema/definitions/internal.py— the new field and the fourordered=Falseflips.backend/infrahub/dependencies/builder/diff/conflicts_enricher.py— wiringdbinto the enricher.backend/tests/component/core/diff/test_ordered_attribute_conflicts.pyand additions totest_conflicts_enricher.py.core/schema/generated/attribute_schema.py,schema/openapi.json,docs/docs/reference/schema/*.mdx.Impact & rollout
ordereddefaults toTrue, preservingexisting order-sensitive behavior for all attributes that do not opt out.
flagged
ordered=False.UpdateSupport.ALLOWED).Checklist
changelog/9764.fixed.md)Summary by cubic
Adds an
orderedflag to attribute schemas so reordering elements inList/JSON arrays no longer triggers merge/rebase conflicts. Actual adds/removes or different counts still conflict.ordered: booleanto attribute schemas (defaulttrue).Listand JSON-array attributes withordered=falseas order-insensitive (multiset). Schema is resolved from the diff branch so a branch’s ownorderedchange applies.ordered=falsefor:enum, dropdownchoices, genericused_by, andrestricted_namespaces.ConflictsEnrichernow takesdb; DI updated. Internals useujsonfor canonicalization.openapi.json; regenerated frontend REST types and refreshed.betterer.results. Docs updated (reference and conflict guide). Tests added for reorder scenarios, including JSON arrays and both recompute paths.Written for commit 8ea3f4a. Summary will update on new commits.