Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

packages/leanix-bridge/src/to-leanix-inventory-dry-run.ts (1)

45-61: Sort the generated artifacts before returning them.

bridge-artifacts.spec.ts snapshots these arrays as JSON, but their order currently depends on whatever traversal order model.elements() and model.relationships() expose. Sorting by likec4Id / likec4RelationId will keep diffs stable if upstream iteration order changes.

♻️ Minimal change

 function buildFactSheetsFromModel(
   model: BridgeModelInput,
   mapping: ReturnType<typeof mergeWithDefault>,
 ): LeanixFactSheetDryRun[] {
   const factSheets: LeanixFactSheetDryRun[] = []
   for (const el of model.elements()) {
     const fsType = getFactSheetType(el.kind, mapping)
     const meta = el.getMetadata()
     const desc = typeof meta['description'] === 'string' ? meta['description'] : undefined
     const tech = el.technology ?? (typeof meta['technology'] === 'string' ? meta['technology'] : undefined)
     factSheets.push({
       type: fsType,
       likec4Id: el.id,
       name: el.title,
       ...(desc !== undefined && { description: desc }),
       ...(tech !== undefined && { technology: tech }),
       ...(el.tags.length > 0 && { tags: [...el.tags] }),
       ...(Object.keys(meta).length > 0 && { metadata: { ...meta } }),
     })
   }
+  factSheets.sort((a, b) => a.likec4Id.localeCompare(b.likec4Id))
   return factSheets
 }
@@
 function buildRelationsFromModel(
   model: BridgeModelInput,
   mapping: ReturnType<typeof mergeWithDefault>,
 ): LeanixRelationDryRun[] {
   const relations: LeanixRelationDryRun[] = []
   for (const rel of model.relationships()) {
     const titleVal = rel.title ?? rel.kind
     relations.push({
       type: getRelationType(rel.kind, mapping),
       likec4RelationId: rel.id,
       sourceLikec4Id: rel.source.id,
       targetLikec4Id: rel.target.id,
       ...(titleVal != null && titleVal !== '' && { title: String(titleVal) }),
     })
   }
+  relations.sort((a, b) => a.likec4RelationId.localeCompare(b.likec4RelationId))
   return relations
 }

Also applies to: 69-80

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@packages/leanix-bridge/src/to-leanix-inventory-dry-run.ts` around lines 45 -
61, The returned arrays must be stably sorted to avoid snapshot diffs: after
building the factSheets array in to-leanix-inventory-dry-run.ts (the const
factSheets: LeanixFactSheetDryRun[] built from model.elements() using
getFactSheetType, el.id, el.title, etc.), sort factSheets by likec4Id before
returning; likewise, after building the relationships array (the array built
from model.relationships() that includes likec4RelationId), sort relationships
by likec4RelationId before returning so the output order is deterministic.

packages/leanix-bridge/src/sync-to-leanix.spec.ts (1)

12-29: Fail the mock on unexpected GraphQL operations.

Returning {} for unknown queries makes the suite permissive: renamed or extra API calls are silently treated as 'not found'. Throwing here keeps the planSyncToLeanix() contract explicit.

🔒 Tighten the mock

       if (
         query.includes('FindFactSheet') &&
         typeof fsName === 'string' &&
         typeof fsType === 'string'
       ) {
         const key = `${fsName}|${fsType}`
         const id = existingByKey.get(key)
         return {
           allFactSheets: {
             edges: id ? [{ node: { id, name: fsName, type: fsType } }] : [],
           },
         }
       }
-      return {}
+      throw new Error(`Unexpected GraphQL query in test: ${query}`)

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@packages/leanix-bridge/src/sync-to-leanix.spec.ts` around lines 12 - 29, The
graphql mock in the test (the async function named graphql) silently returns {}
for unknown queries which hides unexpected GraphQL calls; update the mock so
that after handling the expected FindFactSheet path (using variables['name'],
variables['type'] and existingByKey) it throws an Error for any other
query/operation (including unexpected names or shapes) with a descriptive
message (e.g., include the query string or operation name) so unexpected/renamed
API calls fail the test explicitly.

packages/leanix-bridge/src/sync-to-leanix.ts (1)

207-228: ⚠️ Potential issue | 🟡 Minor

Surface the missing-ID case as an error.

The try-catch from the previous version is removed (fixing API error propagation), but returning null when the mutation succeeds without an ID still causes silent skipping at line 324. A successful mutation that returns no ID is a server-side anomaly that should be surfaced.

🛠️ Proposed fix: throw when mutation returns no ID

 async function createRelation(
   client: LeanixApiClient,
   sourceFactSheetId: string,
   targetFactSheetId: string,
   relationType: string,
   _title?: string,
-): Promise<string | null> {
+): Promise<string> {
   type CreateResult = { createRelation?: { relation?: { id: string } } }
   const mutation = `
     mutation CreateRelation($source: ID!, $target: ID!, $type: String!) {
       createRelation(source: $source, target: $target, type: $type) {
         relation { id }
       }
     }
   `
   const data = await client.graphql<CreateResult>(mutation, {
     source: sourceFactSheetId,
     target: targetFactSheetId,
     type: relationType,
   })
-  return data.createRelation?.relation?.id ?? null
+  const id = data.createRelation?.relation?.id
+  if (!id) {
+    throw new Error(`createRelation returned no ID for ${relationType} (${sourceFactSheetId} → ${targetFactSheetId})`)
+  }
+  return id
 }

With this change, the existing try-catch in syncRelationsToLeanix (line 332-334) will capture all failure modes including this edge case.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@packages/leanix-bridge/src/sync-to-leanix.ts` around lines 207 - 228,
createRelation currently returns null when the GraphQL mutation returns no
relation id, which hides a server-side anomaly; update createRelation to throw
an Error (including context like sourceFactSheetId, targetFactSheetId,
relationType and any returned payload) when data.createRelation?.relation?.id is
missing so callers (e.g., syncRelationsToLeanix) receive and handle the failure
instead of silently skipping; ensure the thrown message is descriptive to aid
debugging.

packages/generators/src/drawio/parse-drawio.ts (2)

186-197: Simplify nested ternary for readability.

The static analysis flags the nested ternary at line 192 and negated conditions. Consider extracting to clearer logic:

♻️ Suggested refactor

 function extractStyleFromTagContent(fullTag: string, maxScan = STYLE_VALUE_SCAN_CHARS): string | undefined {
   const scan = fullTag.slice(0, maxScan)
   const styleDq = scan.toLowerCase().indexOf('style="')
   const styleSq = scan.toLowerCase().indexOf("style='")
-  const useDq = styleDq !== -1 && (styleSq === -1 || styleDq <= styleSq)
-  const styleIdx = useDq ? styleDq : styleSq
-  const quote = styleIdx !== -1 ? (useDq ? '"' : "'") : ''
-  if (styleIdx === -1 || !quote) return undefined
+  const hasDq = styleDq !== -1
+  const hasSq = styleSq !== -1
+  if (!hasDq && !hasSq) return undefined
+  const useDq = hasDq && (!hasSq || styleDq <= styleSq)
+  const styleIdx = useDq ? styleDq : styleSq
+  const quote = useDq ? '"' : "'"
   const valueStart = styleIdx + 7
   const valueEnd = scan.indexOf(quote, valueStart)
   return valueEnd !== -1 ? scan.slice(valueStart, valueEnd) : undefined
 }

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@packages/generators/src/drawio/parse-drawio.ts` around lines 186 - 197, In
extractStyleFromTagContent, replace the nested ternary/negated logic that sets
useDq, styleIdx and quote with clearer stepwise logic: compute scanLower once,
get styleDq and styleSq, if both are -1 return undefined, otherwise choose the
smaller non-negative index (styleIdx) and set quote to '"' if styleIdx ===
styleDq else "'" (or vice versa), then use valueStart = styleIdx + 7 and find
valueEnd as before; update the function to use those explicit branches to
improve readability and eliminate the nested ternary.

---

728-774: Cognitive complexity exceeds threshold (38 vs. 30 allowed).

The function correctly validates bridge IDs and parent chains (addressing the previous review feedback), but its cognitive complexity is flagged by static analysis. Consider extracting the bridge ID validation into a separate helper:

♻️ Suggested extraction for clarity

+/** Try to assign bridge-managed FQN if valid and parent chain matches; returns true if assigned. */
+function tryAssignBridgeFqn(
+  v: DrawioCell,
+  idToFqn: Map<string, string>,
+  usedNames: Set<string>,
+  isRootParent: (parent: string | undefined) => boolean,
+): boolean {
+  const bridgeId = v.likec4Id?.trim()
+  if (!bridgeId || !isValidFqn(bridgeId)) return false
+  const parentFqn = v.parent ? idToFqn.get(v.parent) : undefined
+  const useBridgeId = parentFqn === undefined
+    ? isRootParent(v.parent)
+    : bridgeId.startsWith(parentFqn + '.') && bridgeId.length > parentFqn.length + 1
+  if (!useBridgeId) return false
+  idToFqn.set(v.id, bridgeId)
+  for (const segment of bridgeId.split('.')) usedNames.add(segment)
+  return true
+}
+
 function assignFqnsToElementVertices(
   idToFqn: Map<string, string>,
   elementVertices: DrawioCell[],
   containerIdToTitle: Map<string, string>,
   isRootParent: (parent: string | undefined) => boolean,
   uniqueName: (base: string) => string,
   usedNames: Set<string>,
 ): void {
   const baseName = (v: DrawioCell) => v.value ?? containerIdToTitle.get(v.id) ?? v.id
   const idToVertex = new Map(elementVertices.map(v => [v.id, v]))
   const depth = (v: DrawioCell): number =>
     v.parent == null || !idToVertex.has(v.parent) ? 0 : 1 + depth(idToVertex.get(v.parent)!)
   const byDepth = [...elementVertices].sort((a, b) => depth(a) - depth(b))
-  for (const v of byDepth) {
-    const bridgeId = v.likec4Id?.trim()
-    if (bridgeId && isValidFqn(bridgeId)) {
-      const parentFqn = v.parent ? idToFqn.get(v.parent) : undefined
-      const useBridgeId =
-        parentFqn === undefined
-          ? isRootParent(v.parent)
-          : bridgeId.startsWith(parentFqn + '.') && bridgeId.length > parentFqn.length + 1
-      if (useBridgeId) {
-        idToFqn.set(v.id, bridgeId)
-        for (const segment of bridgeId.split('.')) usedNames.add(segment)
-      }
-    }
-  }
+  for (const v of byDepth) tryAssignBridgeFqn(v, idToFqn, usedNames, isRootParent)
   // ... rest unchanged
 }

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@packages/generators/src/drawio/parse-drawio.ts` around lines 728 - 774, The
assignFqnsToElementVertices function is too complex; extract the bridge-ID and
parent-chain logic into a small helper and also pull out the depth calculation
to reduce nesting and conditionals: create a helper like
isUsableBridgeId(bridgeId: string, v: DrawioCell, idToFqn: Map<string,string>,
idToVertex: Map<string,DrawioCell>, isRootParent: (p?:string)=>boolean) that
encapsulates the checks currently inside the first for-loop (including
isValidFqn, computing parentFqn, and the useBridgeId logic), and another small
helper depth(v: DrawioCell, idToVertex: Map<string,DrawioCell>) to replace the
inline recursive depth; then call these helpers from assignFqnsToElementVertices
(leaving the loop structure intact) to reduce cognitive complexity and improve
readability while preserving behavior.

packages/leanix-bridge/src/sync-to-leanix.ts (1)

422-428: Plan always shows relations as 'create' regardless of manifest state.

The planSyncToLeanix function marks all relations with action: 'create', but syncRelationsToLeanix at line 362 skips creation when rel.external?.[LEANIX_PROVIDER]?.relationId already exists in the manifest. This means the plan may overstate the number of relations to create if the manifest already contains LeanIX relation IDs from a previous sync.

Consider either:

• Accepting a manifest parameter in planSyncToLeanix to filter out already-synced relations, or
• Documenting this behavior in the SyncPlan.relationPlans JSDoc

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@packages/leanix-bridge/src/sync-to-leanix.ts` around lines 422 - 428,
planSyncToLeanix currently sets action: 'create' for every
leanixDryRun.relations entry which overstates creates; change planSyncToLeanix
signature to accept an optional manifest parameter and filter
leanixDryRun.relations before building SyncPlan.relationPlans so any relation
with rel.external?.[LEANIX_PROVIDER]?.relationId in the manifest is skipped (or
marked action: 'none' if you prefer), i.e. update the mapping of
leanixDryRun.relations in planSyncToLeanix to check the provided manifest for
existing relation IDs and only include new relations in the produced
SyncPlan.relationPlans; keep references to planSyncToLeanix,
SyncPlan.relationPlans, leanixDryRun.relations, syncRelationsToLeanix and
LEANIX_PROVIDER to locate and align behavior.