Skip to content

IFC-2113: Fix failure when removing a profile with a many relationship and overriding with same peers#9350

Open
solababs wants to merge 1 commit into
stablefrom
fix-profile-many-rel-override-ifc-2113
Open

IFC-2113: Fix failure when removing a profile with a many relationship and overriding with same peers#9350
solababs wants to merge 1 commit into
stablefrom
fix-profile-many-rel-override-ifc-2113

Conversation

@solababs
Copy link
Copy Markdown
Contributor

@solababs solababs commented May 26, 2026

Why

When a node's cardinality-many relationship is sourced from a profile (peers A, B, C) and the user saves an override containing a peer that also appears in the profile (e.g. [C, D, E]), peer C is silently dropped. Only D and E remain.

The root cause: RelationshipManager.update() reuses the existing Relationship object for C without clearing its is_from_profile=True flag. When NodeProfilesApplier.apply_profiles() runs immediately after (before save()), it sees C as profile-sourced and removes it because C is not in the set of user-set peers.

This PR clears is_from_profile and the source metadata on any reused relationship peer when the user explicitly includes it in their override.

What changed

  • RelationshipManager.update() now clears is_from_profile on reused peers — when a peer from previous_relationships is included in the new override list, is_from_profile is set to False and clear_source() is called before appending to _relationships. This applies to all three reuse branches (node-object, string peer-ID, dict).

  • After the fix, when apply_profiles() runs, the overlapping peer is already in user_set_peer_ids and is not removed. No schema changes, no API changes, no change to NodeProfilesApplier logic.

How to test

uv run pytest backend/tests/component/core/profiles/ -v
# 49 passed (was 47 passed + 2 xfailed)

The previously-xfailed test_get_many_with_profile_relationships_partial_override now passes and validates the fix directly: profile sets things=[T0, T1], user calls things.update([T1, T2]), result is exactly [T1, T2] with no profile source.

Impact & rollout

  • Backward compatibility: The fix only affects the case where a user override includes a peer that was previously profile-sourced. Peers that are in the profile but absent from the override are still removed (existing behavior preserved). Safe to deploy.

Checklist

  • Tests added/updated
  • Changelog entry added (uv run towncrier create ...)
  • External docs updated (if user-facing or ops-facing change)
  • Internal .md docs updated (internal knowledge and AI code tools knowledge)
  • I have reviewed AI generated content

@solababs solababs requested a review from a team as a code owner May 26, 2026 08:31
@github-actions github-actions Bot added the group/backend Issue related to the backend (API Server, Git Agent) label May 26, 2026
@solababs solababs changed the title IFC-2113: Fix profile with many relationships override IFC-2113: Fix removing a profile with a many relationship and overriding with same peers fails May 26, 2026
Copy link
Copy Markdown
Contributor

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

Choose a reason for hiding this comment

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

No issues found across 2 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. The change is a narrowly scoped bug fix that clears the is_from_profile flag on reused relationship peers to prevent them from being incorrectly removed by the profile applier, with a small line count, passing tests, and no impact on API, schema, or infrastructure.

Re-trigger cubic

@solababs solababs changed the title IFC-2113: Fix removing a profile with a many relationship and overriding with same peers fails IFC-2113: Fix failure when removing a profile with a many relationship and overriding with same peers May 26, 2026
@codspeed-hq
Copy link
Copy Markdown

codspeed-hq Bot commented May 26, 2026

Merging this PR will not alter performance

✅ 12 untouched benchmarks


Comparing fix-profile-many-rel-override-ifc-2113 (5c42f3e) with stable (edaeba7)1

Open in CodSpeed

Footnotes

  1. No successful run was found on stable (ce9356b) during the generation of this report, so edaeba7 was used instead as the comparison base. There might be some changes unrelated to this pull request in this report.

return None

async def update(
async def update( # noqa: PLR0915
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.

should either add this to the exceptions in pyproject.toml or refactor the method so the error isn't raised

assert len(node_map) == 1
final_child_one = node_map[child_and_thing_nodes.child_nodes[0].id]

await _validate_node_profile_relationships(
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.

not caused by this PR, but it looks like this function does not correctly validate the case where peers are defined and source_uuid="", which is what we need to test for this case. it looks like it correctly tests that the source of the peer(s) is empty if no ExpectedProfileRelationship for the relationship name, but does not verify that if the ExpectedProfileRealtionship is defined with a source set to the empty string

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.

2 participants