packages/core/src/types/view-changes.ts (1)

37-54: ⚡ Quick win

Require at least one property in change-property payload.

ChangeProperty currently permits { op: 'change-property' }, which creates a valid but meaningless operation. Tightening this contract prevents silent no-ops in downstream handlers.

Suggested type-shape tightening

 export namespace ViewChange {
+  type AtLeastOne<T> = {
+    [K in keyof T]-?: Required<Pick<T, K>> & Partial<Omit<T, K>>
+  }[keyof T]
+
-  export interface ChangeProperty {
-    op: 'change-property'
-    /**
-     * Change title
-     */
-    title?: string
-    /**
-     * Change description
-     */
-    description?: scalar.MarkdownOrString
-    /**
-     * Add or remove tags
-     */
-    tag?: {
-      add?: scalar.Tag | scalar.Tag[]
-      remove?: scalar.Tag | scalar.Tag[]
-    }
-  }
+  export type ChangeProperty = {
+    op: 'change-property'
+  } & AtLeastOne<{
+    /** Change title */
+    title: string
+    /** Change description */
+    description: scalar.MarkdownOrString
+    /** Add or remove tags */
+    tag: {
+      add?: scalar.Tag | scalar.Tag[]
+      remove?: scalar.Tag | scalar.Tag[]
+    }
+  }>
 }

Also applies to: 57-62

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@packages/core/src/types/view-changes.ts` around lines 37 - 54, The
ChangeProperty interface allows an empty payload ({ op: 'change-property' }), so
tighten the type to require at least one of title, description, or tag be
present: replace the current optional-all interface with a discriminated union
or intersection that enforces at least one of { title }, { description }, or {
tag } (referencing ChangeProperty and the tag shape inside it) so consumers
cannot dispatch a no-op; apply the same non-empty requirement to the analogous
interface declared at the other location referenced (lines ~57-62).

packages/diagram/src/editor/actor/state.sync-queue.ts (2)

270-283: ⚡ Quick win

Remove commented-out code.

This substantial commented block appears to be unused exploration code. Consider removing it to improve readability, or if it represents planned functionality, track it in an issue instead.

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@packages/diagram/src/editor/actor/state.sync-queue.ts` around lines 270 -
283, Remove the large commented-out exploratory block that tries to find a
'save-view-snapshot' (the commented calls to find([...context.processing,
...context.syncQueue], p => !!p && isViewChange(p) && p.op ===
'save-view-snapshot') and the subsequent
enqueue.sendTo(typedSystem.diagramActor, { type: 'update.view-bounds', bounds:
snapshot.layout.bounds }) code); either delete it entirely from
state.sync-queue.ts to improve readability or move the logic into a tracked
issue if it represents planned work, leaving no commented implementation in the
file.

---

185-191: ⚡ Quick win

Use optional chaining for cleaner conditional.

The condition current && current._layout === 'manual' can be simplified with optional chaining.

♻️ Proposed fix

           input: ({ context }) => {
              const current = context.history?.head.change.layout
              return ({
-               current: current && current._layout === 'manual' ? current : undefined,
+               current: current?._layout === 'manual' ? current : undefined,
                viewId: context.viewId,
              })
            },

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@packages/diagram/src/editor/actor/state.sync-queue.ts` around lines 185 -
191, The conditional `current && current._layout === 'manual'` should use
optional chaining for clarity; in the input arrow function (the input: ({
context }) => { ... } block in state.sync-queue.ts) replace that expression with
`current?._layout === 'manual'` so the returned object sets current to current
when its _layout equals 'manual', otherwise undefined, leaving viewId unchanged.

packages/diagram/src/editor/actor/actions.ts (1)

83-99: 💤 Low value

Inconsistent action definition pattern.

pushHistory wraps machine.assign in an extra function, unlike saveBeforeEditing (line 67) which directly assigns. This creates an unnecessary closure and diverges from the pattern used elsewhere in this file.

♻️ Proposed fix for consistency

-export const pushHistory = () =>
-  machine.assign(({ context }) => {
+export const pushHistory = machine.assign(({ context }) => {
     const editing = context.editing
     if (import.meta.env.DEV) {
       console.log('pushHistory', editing)
     }
     if (editing) {
       return {
         editing: null,
         history: {
           head: editing.before,
           tail: context.history,
         },
       }
     }
     return {}
   })

Then update the call site in state.editor.ts from pushHistory() to pushHistory.

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@packages/diagram/src/editor/actor/actions.ts` around lines 83 - 99,
pushHistory currently returns a function that calls machine.assign, creating an
unnecessary closure and breaking the file's consistent action pattern (see
saveBeforeEditing which directly assigns). Change pushHistory to be the direct
result of machine.assign (i.e., export const pushHistory = machine.assign(...))
and update the call site in state.editor.ts to use pushHistory (no parentheses)
so the action is referenced consistently with other actions like
saveBeforeEditing.

packages/diagram/src/editor/actor/setup.ts (1)

2-8: 💤 Low value

Consider consolidating imports from the same module.

Multiple separate import statements from ./types could be combined into a single statement.

♻️ Proposed consolidation

-import { hotkey } from './hotkey'
-import type { EditorActorEvent } from './types'
-import type { EditorActorContext } from './types'
-import type { EditorActorEmitedEvent } from './types'
-import type { EditorActorInput } from './types'
-import type { EditorActorStateTag } from './types'
+import { hotkey } from './hotkey'
+import type {
+  EditorActorContext,
+  EditorActorEmitedEvent,
+  EditorActorEvent,
+  EditorActorInput,
+  EditorActorStateTag,
+} from './types'

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@packages/diagram/src/editor/actor/setup.ts` around lines 2 - 8, Multiple type
imports (EditorActorEvent, EditorActorContext, EditorActorEmitedEvent,
EditorActorInput, EditorActorStateTag) are imported separately from the same
module; consolidate them into a single import statement that imports all these
types together (keep the other import from 'xstate' as-is), updating the
existing import lines in setup.ts to use one combined import for the listed
EditorActor* types.

packages/diagram/src/search/Search.tsx (1)

130-139: 💤 Low value

Mark props as read-only for type safety.

Per static analysis, the props type should be marked as read-only to prevent accidental mutation.

🔧 Proposed fix

-function SearchOverlayBody({ searchActorRef }: { searchActorRef: SearchActorRef }) {
+function SearchOverlayBody({ searchActorRef }: Readonly<{ searchActorRef: SearchActorRef }>) {

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@packages/diagram/src/search/Search.tsx` around lines 130 - 139, The props for
SearchOverlayBody should be marked readonly to prevent mutation; update the
function signature for SearchOverlayBody so the parameter type uses a readonly
prop type (e.g., wrap the inline props type in Readonly<...> or declare a
Readonly interface) for the searchActorRef prop, ensuring the type references
SearchActorRef and that the JSX usage of SearchPanelContent and ref remain
unchanged.

packages/language-server/src/model-change/viewChange.spec.ts (1)

67-614: ⚡ Quick win

Add one idempotent/no-op change-property regression test.

Current coverage is great for mutating edits, but it should also assert behavior when the requested property change produces no text edit (e.g., same title/description or removing a non-existing tag).

As per coding guidelines, “Aim to cover new features with relevant tests; keep test names descriptive.”

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@packages/language-server/src/model-change/viewChange.spec.ts` around lines 67
- 614, Add a new test under the existing "change-property" describe that
verifies idempotent/no-op edits: use testDoc to create a fixture, capture the
initial read(), then call change(...) (the change helper) with a no-op
change-property (e.g., setting title to the same value and/or removing a tag
that isn't present) and assert that read() is strictly unchanged and that no
text edits occurred; name the test something like "should be idempotent for
no-op changes" and place it alongside the other it(...) cases in the viewChange
spec.

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes