PR State:
merged
PR:
#43
Commit:
7d691c8d07e80c4841fc2a4b19dcfcf18e2fae18
[IMP] P16 review supplement: fix review findings + Brain editor UX polish (#2)
* [FIX] viin_ai_ops: null-guard ListRenderer.setDefaultColumnWidths against intermittent DOM race
Intermittent TypeError "Cannot read properties of null (reading 'style')"
in core Odoo 17 ListRenderer.setDefaultColumnWidths() surfaces on the
Action Proposals list when the view re-renders with zero records (isEmpty
toggle races thead commit to DOM, querySelector returns null).
Adds a defensive monkey-patch in viin_ai_ops that overrides the method
with an identical but null-safe implementation: if headerEl is null the
column iteration step is skipped instead of crashing. This matches the
upstream fix strategy applied in Odoo 18 via useMagicColumnWidths.
BACKPORT FROM: Odoo 17 core list_renderer.js (upstream fixed in 18.0)
REMOVE WHEN UPGRADING TO: 18.0
* [FIX] viin_ai_editor: surface UserError message in AI block error state
Odoo 17 RPCError shape wraps the real UserError text in err.data.message
while err.message is always the opaque "Odoo Server Error" wrapper.
The catch in AiBlock._generate now reads err.data.message first so users
see the actionable UserError (e.g. "No AI agent configured") instead of
the unhelpful wrapper.
- ai_block.js: prioritise err.data.message || err.message || fallback _t()
- conventions.md: add JSON-RPC error handling convention (SSOT: chat_panel.js)
- test_ai_block_state_machine.js: add 2 cases covering RPCError shape and
plain-Error fallback
* [IMP] viin_web_editor,viin_brain,viin_ai_brain: dedup powerbox native commands (WI-007)
Introduce a generic suppress-native registry in viin_web_editor so downstream
modules can declaratively remove native OdooEditor/Wysiwyg powerbox commands that
they replace, without the engine hardcoding any consumer-specific names.
Changes:
- viin_web_editor/powerbox_registry.js: export new registry
viin_web_editor.powerbox_suppress_native with suppressor shape
{matcher, contextGate?}
- viin_web_editor/wysiwyg_patch.js: filter upCommands (native) through all
registered suppressors before concatenating extra commands; exceptions inside
suppressors are caught per-entry (fail-safe)
- viin_brain/slash_commands.js: register five suppressors (headings/quote/code/
bullet/numbered, all context-gated to .o_brain_editor_body_edit); fix
brain_bullet_list and brain_numbered_list to use toggleList (identical to
native, preserves toggle-off semantics)
- viin_ai_brain/powerbox_commands.js: register suppressor for native ChatGPT
(fa-magic) gated on bridge.available=true
- docs/decisions/wi-007-powerbox-suppress-native.md: new append-only WI,
Refines ADR-010
* [FIX] viin_ai_brain: multi-company filter agent lookup (B1)
Extract the Knowledge Base Assistant agent lookup into a single SSOT
helper `viin_ai_brain/controllers/common.py::find_kba_agent()` that
enforces `company_id in [env.company.id, False]` on every search.
Before this fix wysiwyg.py `_get_agent()` searched by name only
with no company filter, leaking cross-tenant agent records. ai_block.py
already had the filter but in a local function. Both controllers now
import `find_kba_agent` from `controllers/common.py`, eliminating the
duplicate logic and the security gap.
* [IMP] viin_ai_brain: topic-based agent lookup + CTA (MED-2)
Update `controllers/common.py::find_kba_agent` to resolve the KBA agent
via the `viin_ai_brain.topic_brain_assistant` topic xmlid instead of the
display name. This makes lookup name-independent: renaming the agent for
i18n or branding purposes does not break any controller path.
When no agent with the topic is found, raises UserError with an
actionable CTA pointing to AI > Configuration > Agents
(viin_ai_agent.action_viin_ai_agent).
Frontend changes:
- ai_bridge_service.js: pass topicXmlId instead of hard-coded topicName
for ChatPanel so the chat panel can resolve the topic server-side.
- ai_editor_backend_service.js: remove "Knowledge Base Assistant"
hard-coded fallback for agent_name; server is the SSOT (empty string
sentinel on RPC success avoids masking real lookup failures).
Tests (Odoo native TransactionCase, @tagged post_install):
- TestKbaAgentLookupMultiCompany: B1 isolation - company A env cannot
resolve company B's KBA agent; global agents (company_id=False) are
visible from any company; missing-company agent raises UserError+CTA.
- TestKbaAgentTopicLookup: rename-safe lookup (MED-2); detaching topic
raises UserError+CTA; wysiwyg endpoint surfaces CTA end-to-end.
- Updated TestBrainBridgeCommon to attach the KBA topic to the fixture
agent, and updated _ensure_brain_agent in tour tests likewise.
- Updated test_commit_attribution_raises_when_no_kba_agent: detach topic
(not rename) to simulate missing agent under topic-based lookup.
Verified: 0 failed, 0 error of 57 tests (wave_wid_test ephemeral DB).
* [IMP] viin_brain: fix AI badge wording to reflect installed-not-ready semantics
Badge text changed from "AI ready" to "AI features available" and tooltip
updated to "AI features are installed - configure a provider & agent to use
them." to avoid implying the AI is operational before a provider and agent
are configured (Option B wording-only fix, WI-E).
* [DOC] docs: WI-008 PR #43 deferred follow-ups tracking + roadmap corrections
Create docs/decisions/wi-008-pr43-deferred-followups.md (append-only WI):
- WI-F (MED-3): Provider Test Connection button -> viin_ai_base Layer 0 backlog
- WI-G: L2 active-default policy (needs ADR-011), L3 dashboard CTA (W12 P-OPS-4),
L4 admin empty-state guidance (Phase 5/onboarding)
- WI-007-onboarding: Phase 5 ADR + wizard (M4 gate); seed agent stub explicitly
excluded (model_id required+restrict constraint); CTA error already done in WI-D2
- B2: CRDT pitch guidance - interim relay is NOT OT real-time collab (ADR-002 intact)
- B3: SQL template count corrected to 15 (roadmap said 11 pre-viin_ai_crm baseline;
review cited 13; correct per CEO 2026-06-06 = 15)
Update docs/roadmap.md:
- Fix SQL template count 11 -> 15 with WI-008 B3 reference
- Add Layer 0 backlog note after M1 gate (WI-F, WI-G L2)
- Add AI Onboarding wizard item to Phase 5 W16-W19 (gated M4)
- Add WI-005, WI-006, WI-008 rows to ADR/WI cross-reference table
- Mark ADR-010 DONE 2026-06-04; add ADR-011 candidate for active-default policy
Update docs/decisions/README.md:
- Add wi-008 row to WI index
- Update date to 2026-06-06
* [FIX] viin_ai_brain,viin_ai_editor,viin_brain,viin_web_editor: eslint prettier formatting on WI changes
* [REF] viin_ai_brain: Command.* in WI-D tests + drop dead chat prop (review LOW)
* [FIX] viin_brain: powerbox suppressor context-gate timing (Phase V blocker)
Root cause: _getPowerboxOptions() runs synchronously inside startEdition().
The DOM markers (.o_brain_editor_body_edit[data-page-id]) are applied in the
startWysiwyg callback AFTER startEdition() resolves, so _isBrainContext()
always returned false during powerbox build - all 5 suppress-native rules
were no-ops, causing 7 native commands to appear twice (Heading 1/2/3,
Text, Numbered list, Quote, Link).
Fix: set wysiwyg.__brainPageContext = true on the wysiwyg instance BEFORE
calling startEdition() in page_editor.js. Update _isBrainContext() to check
this early-signal flag (Tier 1) before falling back to the DOM closest()
check (Tier 2). Flag is Brain-PageEditor-only; mail/website editors are
unaffected.
Bonus: deduplicate powerbox "AI" category registrations. Three WI phases
(WI-A1, WI-B3, WI-007) each registered a separate category key all named
"AI", producing three UI separators. Guard each registration behind a check
for all known AI category keys so only one "AI" category appears in the
powerbox when both viin_ai_editor and viin_ai_brain are installed.
* [FIX] viin_web_editor,viin_brain,viin_ai_brain: suppress native powerbox at merge point + Text/Link + AI category (Phase V)
- wysiwyg_patch.js: add Tier 2 powerboxFilters injection to suppress OdooEditor-level
commands (Heading 1/2/3, Bulleted list, Numbered list, Text) that bypass
_getPowerboxOptions and cannot be caught by Tier 1 alone. Use _viinCustom sentinel
to distinguish consumer commands from native ones so suppressors never accidentally
remove Brain's own commands that share the same fontawesome icon.
- slash_commands.js: add viin_brain.suppress_text (fa-paragraph) and
viin_brain.suppress_link (fa-link) suppressors for the native Text and
Link/Button commands that brain_paragraph and brain_external_link replace.
- brain_editor_registry_commands.js: document that key "brain_ai" is intentional
(viin_ai_editor uses .add("AI") unconditionally; using the same key here would
throw on duplicate). Powerbox.open() deduplicates categories by name at runtime.
- powerbox_commands.js (viin_ai_brain): same documentation for key "brain_ai_write".
- wi-007-powerbox-suppress-native.md: update to describe two-tier architecture,
add Text/Link suppressor table rows, add AI category safety analysis.
* [FIX] viin_ai_brain: group AI powerbox commands under single category (Phase V)
Root cause: _t("AI") at module-evaluation time returns LazyTranslatedString
*objects* (extends String). Different _t() calls across modules produce
different object instances never === each other. Powerbox._groupCommands()
matches command.category === category.name with strict equality, so the match
always fails for cross-module LazyTranslatedString instances. Each AI command
fell into the remaining-categories path and got its own "AI" header, rendering
3 separate "AI" separators instead of one.
Fix: use plain string literal "AI" (not _t("AI")) for both category name and
command category across all 3 AI-contributing modules. Plain strings compare
correctly with ===. "AI" is locale-invariant so no i18n value is lost.
All 3 modules now register under canonical key "AI":
- viin_ai_editor: add("AI", ..., {force:true}) - prevents DuplicatedKeyError
when brain/ai_brain registers first
- viin_ai_brain, viin_brain: guarded by !contains("AI") - avoid overwrite
warning when viin_ai_editor loads first
WI-007 decision doc updated to document the real root cause and the fix.
* [FIX] viin_brain: vault page resequence no longer clears the tree (P16 review)
Root cause: _reorderPage called _invalidateParent(null) after the ORM writes.
_invalidateParent deletes childrenByParent.get(parentId) then re-fetches only
when parentId is in expandedIds. The root parent (null) is never in
expandedIds, so the root list was permanently deleted and the sidebar showed
"No page in this vault" until the user navigated away or reloaded the page.
Server data was saved correctly — only the UI was affected.
Fix: optimistic reorder. Swap sequence values directly in the in-memory
pageCache before awaiting the ORM writes, re-sort the affected parent's child-id
array in childrenByParent using the updated sequences, then bump the flatRows
cache. The list is never cleared between the drop event and the server
response. A try/catch around the ORM writes restores consistency via
_invalidateParent + an explicit _loadRoots call only in the error path.
A new QUnit unit test (no-mount harness, same pattern as the WI-8 suite) pins
the regression: after _reorderPage the root list must still exist, contain both
page ids, and be re-sorted by the new sequences.
* [IMP] viin_brain: Brain editor UX polish - editor border, powerbox default style, placeholder hints, clickable breadcrumb (P16 review)
Issue 2 (editor border): fix specificity loss in .o_brain_editor_body_edit
note-editable border override. Add compound selector &.note-editable + !important
backstop so Brain wysiwyg canvas never shows the textarea-like border from
web_editor.backend_assets_wysiwyg regardless of stylesheet load order.
Issue 3 (powerbox style): remove Brain-specific powerbox SCSS overrides
(_powerbox.scss). The custom grey background / washed-out description text
diverged from Odoo default. Revert to Odoo default white-background crisp
powerbox. Command injection via wysiwyg_patch.js / powerboxCommandsRegistry
is completely unaffected - only visual overrides removed.
Issue 4 (placeholder hint): wire OdooEditor native placeholder option in
_buildWysiwygOptions() so empty pages show "Type / for commands, [[ to link
a page, @@ to mention" hint via .oe-hint::before. Also add
o_brain_editor_placeholder class to the editable in startWysiwyg() as CSS
fallback for :empty:not(:focus)::before defined in _app_shell.scss.
Issue 5 (breadcrumb + header lean): make vault breadcrumb segment clickable
(role=button, tabindex=0, cursor pointer, hover highlight, onBreadcrumbVaultClick
handler calls onVaultSelect prop bound from BrainApp.onVaultSelect). Tighten
header layout: margin-bottom 16px->10px, breadcrumb mb 8px->4px, sep padding
0 6px->0 4px, title font 28px->26px, icon 24px->22px, padding 4px->2px 0.
* [FIX] viin_brain: show editor placeholder hint (CSS cascade fix, P16 review)
Drag-handle rule `> p::before { content: "⋮⋮" }` was overriding
`.oe-hint::before { content: attr(placeholder) }` due to equal specificity
and source order. Fix: narrow the drag-handle selector to
`> p:not(.oe-hint)` so the hint paragraph is excluded from the drag-handle
block entirely. Also add explicit `content: attr(placeholder)` to the
`.oe-hint::before` rule to ensure it always wins regardless of cascade order,
and bump opacity from 0.38 to 0.45 for better readability.
* [FIX] viin_brain: eslint prettier formatting on P16 UI fixes
* [FIX] viin_brain,viin_ai_brain: scope Brain powerbox commands to Brain editor context (P16 review)
Brain-specific powerbox commands were registered globally and leaked into
every editor that loads the powerbox patch (mail templates, CRM description,
website builder, etc.), causing UI noise and potential runtime errors when
Brain-specific callbacks (wikilink, embed, database, record mention) fire
outside a Brain page.
Fix: add isDisabled(wysiwyg) => !isBrainContext(wysiwyg) to all Brain-specific
commands so they are hidden by the Powerbox engine outside a Brain editor.
isBrainContext uses a two-tier check (wysiwyg.__brainPageContext early-signal
flag + DOM .o_brain_editor_body_edit[data-page-id] fallback) identical to the
existing WI-C suppress-native contextGate.
SSOT: extract shared predicate to viin_brain/static/src/utils/brain_context.js
(web.assets_backend eager bundle, no @web_editor dep). Three consumers:
- slash_commands.js (wysiwyg lazy bundle) - all 14 Brain commands gated
- brain_editor_registry_commands.js (eager) - brain_ai_block stub gated
- viin_ai_brain/registries/powerbox_commands.js - brain_ai_write + brain_ai_chat
now require both Brain context AND bridge.available
brain_ai_write / brain_ai_chat: previous _isDisabledWhenNoBridge only checked
bridge availability; these commands also leaked into non-Brain editors when
bridge was active. New _isDisabledWhenNoBridgeOrOutsideBrain combines both.
Not gated: viin_ai_editor ai_block command is a general AI editor feature
(not Brain-specific); its isDisabled already gates on backend.available.
ai_block_conflict_dedup.js WI-C dedup mechanism preserved unchanged.
WI-007 suppress-native contextGate logic preserved unchanged.
* [FIX] viin_ai_editor: eslint prettier trailing comma
* [FIX] viin_ai_brain: address Codex review on PR #43 supplement
Two P2 findings from the automated Codex review:
1. find_kba_agent preferred global over company-specific agents.
order='company_id desc' sorts SQL NULLs (global, company_id=False)
FIRST under PostgreSQL DESC, so a company with its own KBA agent was
still routed through the shared global one. Replaced with a two-step
preference (company-specific search first, global fallback) - DB-agnostic
and provably correct. Added TestKbaAgentCompanyPreference regression tests
(company-specific wins; global still used as fallback).
2. Native ChatGPT (fa-magic) powerbox command was suppressed in EVERY
editor once viin_ai_brain was installed, because the suppressor gated only
on bridge.available (globally true). In non-Brain editors (mail/CRM/website)
the replacement brain_ai_write is itself disabled, leaving no AI write
command. Added isBrainContext(wysiwyg) to the suppression contextGate so
the native command is only replaced inside Brain editors.
Branch:
17.0