Skip to content

fix: pass SchemaBranch when parsing constraint paths in HFID derivation#9322

Merged
iddocohen merged 3 commits into
stablefrom
ic-fix-relationship-only-uniqueness-constraint
May 22, 2026
Merged

fix: pass SchemaBranch when parsing constraint paths in HFID derivation#9322
iddocohen merged 3 commits into
stablefrom
ic-fix-relationship-only-uniqueness-constraint

Conversation

@iddocohen

@iddocohen iddocohen commented May 21, 2026

Copy link
Copy Markdown
Contributor

Why

Creating a node with a single-element relationship-only uniqueness_constraints entry (e.g. [["parent"]]) crashes schema processing with AttributeError: 'NodeSchema' object has no attribute 'get' and returns a 500. The docs list relationship-only constraints as supported syntax, so the implementation and docs disagree. The bug has been present since at
least 0.16.1.

Goal: Make relationship-only single-element uniqueness constraints process without crashing.

Non-goals:

  • Auto-promoting a relationship-only constraint to an HFID (HFID logic only triggers when schema_path.is_type_attribute is true, which a pure relationship path is not — so the constraint correctly stays as a constraint and isn't lifted to HFID).
  • Reconciling whether relationship-only single-element constraints should be a first-class HFID source.

Closes #4483

What changed

Behavioral changes

  • Schema processing no longer crashes when a node defines uniqueness_constraints: [["<relationship>"]] with no other entries.
  • Mixed constraints like ["parent", "name__value"] were already working — that path skipped the broken branch via the if len(constraint_paths) > 1: continue filter, which is why the bug only surfaces for the single-element relationship case.

Implementation notes

  • One-line fix in _derive_human_friendly_id: pass schema=self (the SchemaBranch) to node.parse_schema_path instead of schema=node. parse_schema_path treats its schema argument as a SchemaBranch and calls .get(name=..., duplicate=True) on it when resolving a relationship peer; passing the NodeSchema instead made that call raise.
  • The other two parse_schema_path call sites in the same file already pass schema=self, so this was a one-spot typo, not a systemic issue.

What stayed the same

  • parse_schema_path is unchanged.
  • No API, GraphQL, or DB schema changes.
  • HFID derivation rules are unchanged — relationship-only constraints still do not auto-promote to HFID.

How to review

  • backend/infrahub/core/schema/schema_branch.py — the one-line fix at the _derive_human_friendly_id call site.
  • backend/tests/unit/core/schema/test_schema_branch.py — new test class that builds a hierarchical schema with uniqueness_constraints=[["parent"]] and asserts process_human_friendly_id does not raise. The test fails on stable with the original AttributeError.

No risky areas — the change strictly narrows the failure surface; mixed-constraint and attribute-only paths were already exercising the correct argument.

How to test

# Run the new unit test
uv run pytest backend/tests/unit/core/schema/test_schema_branch.py::TestDeriveHumanFriendlyIdWithRelationshipOnlyConstraint -v

# Full schema unit suite to confirm no regressions
uv run pytest backend/tests/unit/core/schema/ backend/tests/unit/core/test_schema_defaults.py

To reproduce on stable before the fix, load a schema like:

nodes:
  - name: Site
    namespace: Testing
    inherit_from: [TestingLocation]
    parent: TestingCountry
    uniqueness_constraints:
      - [parent]

Schema processing raises AttributeError: 'NodeSchema' object has no attribute 'get' → HTTP 500. With this PR the same schema loads cleanly.

Impact & rollout

  • Backward compatibility: None broken. Schemas that previously crashed now load; schemas that previously worked are unaffected.
  • Performance: None. Single attribute access change inside a path already taken on every schema reload.
  • Config/env changes: None.
  • Deployment notes: Safe to deploy. No migrations, no coordinated release needed.

Checklist

  • Tests added/updated
  • Changelog entry added (changelog/4483.fixed.md)
  • External docs updated — N/A, docs already describe the intended (now-working) behavior
  • Internal .md docs updated — N/A
  • I have reviewed AI generated content

Summary by cubic

Fixes a crash when a node uses a single-element relationship-only uniqueness_constraints (e.g., [["parent"]]) by passing the SchemaBranch to parse_schema_path during HFID derivation. Adds a fixture-based unit test that processes a simple Car/Person schema to verify the constraint resolves without error and does not promote to an HFID.

Written for commit 8e29c5a. Summary will update on new commits. Review in cubic

_derive_human_friendly_id was passing the NodeSchema as the `schema`
argument to parse_schema_path, but that parameter is treated as a
SchemaBranch (the function calls .get(name=..., duplicate=True) on it).
With a single-element relationship-only uniqueness_constraint (e.g.
[["parent"]]), the relationship branch runs and crashes with
AttributeError: 'NodeSchema' object has no attribute 'get'.

Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]>
@github-actions github-actions Bot added the group/backend Issue related to the backend (API Server, Git Agent) label May 21, 2026

@cubic-dev-ai cubic-dev-ai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No issues found across 3 files

Confidence score: 5/5

  • Automated review surfaced no issues in the provided summaries.
  • No files require special attention.

Shadow auto-approve: would auto-approve. This is a one-line fix that passes the correct argument (SchemaBranch) to parse_schema_path, resolving a crash for single-element relationship-only uniqueness constraints without changing any logic, API, or database schema, and includes a comprehensive unit test that fails on the original code.

Re-trigger cubic

@codspeed-hq

codspeed-hq Bot commented May 21, 2026

Copy link
Copy Markdown

Merging this PR will not alter performance

✅ 12 untouched benchmarks


Comparing ic-fix-relationship-only-uniqueness-constraint (8e29c5a) with stable (74f25ba)

Open in CodSpeed

@iddocohen iddocohen marked this pull request as ready for review May 21, 2026 11:27
@iddocohen iddocohen requested a review from a team as a code owner May 21, 2026 11:27

@cubic-dev-ai cubic-dev-ai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

0 issues found across 2 files (changes from recent commits).

Shadow auto-approve: would auto-approve. This one-line fix corrects a typo where the wrong schema object was passed to parse_schema_path, resolving a crash for single-element relationship-only uniqueness constraints without affecting existing behavior, and it includes a targeted test that validates the fix.

Re-trigger cubic

@ajtmccarty ajtmccarty left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

hopefully this isn't too offensive, but I pushed two commits to your branch to add a fixture (that can be used in other unit tests) and call process() on SchemaBranch instead of just some of the methods that process() calls. I thought it would be easier than describing the changes, but I can do that in the future if you'd prefer

@iddocohen iddocohen merged commit e3b9f4d into stable May 22, 2026
64 checks passed
@iddocohen iddocohen deleted the ic-fix-relationship-only-uniqueness-constraint branch May 22, 2026 08:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

group/backend Issue related to the backend (API Server, Git Agent)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

bug: schema fails to load when a schemanode defines a single uniqueness constraint on a relationship, without an HFID

2 participants