swarm-assistent/docs/reviews/2026-08-22-review-1.md

Project Review — 2026-08-22 (1)

Scope: Swarm Assistent 0.11.8 (client Assets/assistent.js, C# pipeline, packs, HTML/CSS). Includes a verification of the 0.11.8 ship (a5e96f7).

Prior Reviews Summary

Based on the last 3 review files analysed in Phase 0.

Still Open (carried forward)

None.

Resolved Since Last Review

None. (no prior docs/reviews/ files)

---

0.11.8 verification

Checked against the ship notes: session_exact, slim /debug ask, generate-only-for-frames.

Correct

• ListPacks skips hidden: true; LoadPackPrompt still loads enabled hidden packs (debug_explain.json has hidden: true, enabled: true).
• Slim debug skips prefs, skills, identity, RAG retrieve, tool hops, and memory/pref/persona writes; Exact still loads when includeBase is false.
• Client fromDebug sends pack debug_explain, includeBase: false, skipAppendUser, empty skills; dump stays a system note.
• Single doParams body; shouldRememberSessionParam remembers when the user asked or the value differs from Exact.
• userImpliesGenerate has no noun-only fallback; userIsChatNotFrame strips generate on thanks/trivia; willGen no longer ORs the auto-generate checkbox.
• Auto-critique / auto-vision HTML defaults remain unchecked (0.11.4 opt-in).
• Negative pass-through and variant-interrupt epoch guard still in place.

Not fully correct (filed below)

• Nested Krea-prep / empty-patch retry sendChat is a no-op while state.busy is true (vision/critique hops are exempt; these are not).
• fromDebug returns after rememberLastPatch / interrupt / maybeVisionHop.
• Model actions:["generate"] still runs Generate unless userIsChatNotFrame matches.
• shouldHonorLookAt still honors unsolicited look_at on non-generate turns.

---

Phase 1: Code Quality

SOLID

Assets/assistent.js is a single IIFE (~9600 lines) owning chat, board, patches, inventory, personas, cards, and settings. C# is split into partial files on SwarmAssistentExtension, which is appropriate for a SwarmUI extension. No extra SOLID tasks beyond the concrete bugs below.

Performance

• Every RAG retrieve loads the full memories table plus embeddings, then filters in C# — AssistentMemory.cs line 750.

Correctness & Bugs

• Nested sendChat from handleReplySideEffects (Krea EN prep, empty-patch retry) hits the state.busy gate and returns without sending — Assets/assistent.js lines 8622–8624, 8104–8115, 8169–8181.
• /debug side effects run before the Q&A early return — lines 8120–8160.
• C# TryParsePatch returns the first fence; JS extractPatch keeps the last — AssistentPatch.cs line 97 vs Assets/assistent.patch.js line 88.
• ResolveEnabledSkills ignores skills: [] (Count > 0) so the user cannot disable all skills — AssistentConfig.cs line 1474. Slim debug is unaffected because it skips the skills layer.
• doInterruptNow bumps chatEpoch without clearInFlightUi; finishOk then bails and leaves state.busy true — Assets/assistent.js lines 4629–4635, 8867–8868.
• Failed RunToolHop (follow == null) breaks the hop loop, dropping sibling tools (e.g. empty memory_get kills search_civitai) — AssistentChatPipeline.cs lines 237–239.
• Overlay JSON writes never take AssistentConfig._lock (the field is unused) — AssistentConfig.cs line 18.
• Wanted YAML is load–mutate–WriteAllText with no lock — AssistentWanted.cs lines 46–77.
• Patch key lists diverge: JS has persona_clone / persona, neither list has scheduler (but applyPatch writes scheduler) — AssistentPatch.cs lines 12–25, Assets/assistent.patch.js lines 8–19, Assets/assistent.js line 4327.

Code Quality

No extra tasks. Duplicated help strings (JS fallback vs ui.json) are filed under UX.

---

Phase 2: Logical Consistency

Domain & Application Layer

No layered DDD. Pack/core contracts vs client behavior are the real domain rules.

Data Flow

• Server hops parse the first JSON fence; the UI applies the last. Hops/ApplyMemoryActions can follow a weak fence while Generate uses a later one.
• includeBase only gates core.md; Exact/live still flow on debug turns (intended).

State Management

• fromDebug can overwrite state.lastPatch and start a vision hop before the early return.
• skipAppendUser persists an assistant explanation with no matching user turn in state.history — next chat turns see a dangling assistant message.

Consistency

• write_prompt.md always demands a JSON patch with prompt+negative; core.md output contract shows a generate example as mandatory, while later saying “Pure Q&A: omit the JSON patch”. Client auto-apply will still write Swarm fields if the model emits a prompt patch on chat.
• shouldHonorLookAt returns true whenever there is look_at and no generate trigger — contradicts 0.11.4 / pack “look only if asked”.
• wantsGen trusts model actions:["generate"] unless userIsChatNotFrame (narrow). synthesizePatchAfterEmptyFence can invent actions:["generate"] when the reply has an empty ### JSON Patch heading even if the user did not ask for a frame.

---

Phase 3: UI/UX

Usability

• Loading spinner, Stop, empty chat/board states exist.
• Chat clear confirms; Clear Init+Mask in the board ⋯ menu does not.
• /debug ask dump as a system note (no second user bubble) is correct.

Visual & Consistency

No token/theme issues filed. Status and health use text plus color.

Interaction & Feedback

• Interrupt from the Stop button clears UI; interrupt from a model patch does not (see busy-stuck bug).
• README slash table and /pack error string lag behind Config/_base/ui.json.

Accessibility

• Composer #sa_input has only a placeholder (no accessible name).
• Persona/pack/model <select>s use title only.
• Main subtabs are a tablist without role="tab" / aria-selected (setView toggles CSS class only).
• .sa-board / .sa-slot set outline: none with no :focus-visible replacement; board is tabindex="0".
• #sa_status is not aria-live.

---

Tasks

Only actionable issues are listed here. Carried-forward items from prior reviews come first (marked with source), followed by net-new findings.

• 1. [Bug] Exempt fromPromptEnRetry and fromEmptyPatchRetry from the state.busy gate (same as vision/critique). Nested Krea-prep and empty-patch retry currently return immediately; the “Готовлю промпт…” note is shown and Generate is skipped. — Assets/assistent.js line 8623 (already resolved via isContinuationTurn)
• 2. [Bug] Move the fromDebug early return to the top of handleReplySideEffects (after extract). Today debug can rememberLastPatch, doInterruptNow, and maybeVisionHop before the Q&A return. — Assets/assistent.js line 8151 (already resolved: return immediately after extractPatch)
• 3. [Bug] Align C# TryParsePatch with JS extractPatch: prefer the last terminal/strong fence, not the first match. Server hops otherwise act on a weak fence while the UI applies a later one. — AssistentPatch.cs line 97
• 4. [Logic] shouldHonorLookAt must not return true for unsolicited look_at on ordinary/write turns. Honor only user /look, vision packs, or auto-critique/hop. — Assets/assistent.js line 1184
• 5. [Bug] Do not start Generate from a model actions:["generate"] or from synthesizePatchAfterEmptyFence unless the user turn is a frame (userImpliesGenerate / explicit /gen). Empty ### JSON Patch on chat currently synthesizes generate. — Assets/assistent.js lines 915 and 8164
• 6. [Bug] doInterruptNow (patch interrupt action) must call clearInFlightUi like the Stop button. Epoch bump inside finishOk currently leaves state.busy stuck. — Assets/assistent.js line 4629
• 7. [Bug] Honor clientSkills even when empty: if (clientSkills is not null) (null = defaults, [] = none). Unchecking every skill currently reloads defaultOn. — AssistentConfig.cs line 1474
• 8. [Bug] On RunToolHop returning null, skip that tool and try the next NextToolHop instead of break. Empty memory_get currently aborts search_civitai / inventory on the same patch. — AssistentChatPipeline.cs line 237
• 9. [Logic] Make write_prompt.md and core.md match ordinary: fenced JSON only on frame turns; Q&A is prose only. The “mandatory JSON patch with prompt+negative” line fights auto-apply on chat. — Config/_base/packs/write_prompt.md line 8
• 10. [Bug] Sync patch key lists: add scheduler to JS PATCH_KEYS and C# PatchKeys; add persona_clone / persona to C#. A scheduler-only (or clone-only) fence is dropped. — Assets/assistent.patch.js line 8
• 11. [Performance] Filter RAG SELECT by persona chain (and/or FTS id list) instead of scanning every memory row + embedding on each chat. — AssistentMemory.cs line 750
• 12. [Bug] Use AssistentConfig._lock around overlay read-modify-write (SaveSettings, control shelves) or remove the unused lock. Concurrent API calls can tear JSON. — AssistentConfig.cs line 18
• 13. [Bug] Lock Wanted YAML load/merge/save (or write atomically). Concurrent enqueues can drop entries. — AssistentWanted.cs line 46
• 14. [Logic] For skipAppendUser, persist a short history marker (/debug ask) with the assistant reply, or omit both. Orphan assistant turns pollute the next LLM context. — Assets/assistent.js line 8775
• 15. [Accessibility] Name the composer (aria-label on #sa_input) and header selects; give main subtabs role="tab" + aria-selected in setView. — Tabs/Text2Image/Assistent.html line 93
• 16. [Accessibility] Restore a :focus-visible ring on .sa-board and .sa-slot (they set outline: none; board is keyboard-focusable). — Assets/assistent.css line 95
• 17. [UX] Mark #sa_status as role="status" / aria-live="polite" so busy/error text is announced. — Tabs/Text2Image/Assistent.html line 108
• 18. [UX] Confirm before Clear Init+Mask (chat clear already confirms). — Assets/assistent.js line 9425
• 19. [UX] Sync README slash table and /pack error status with Config/_base/ui.json (/new, /history, /persona *, ordinary|card|persona). — README.md line 115