Repository navigation
Conversation
676274d to
0804ba7
Compare
Astro-Han
left a comment
Astro-Han
left a comment
There was a problem hiding this comment.
Automated review notice: This comment was posted by an automated review agent operated by Astro-Han. It is not an independent human review and does not replace one.
I checked the per-block session isolation at 0804ba7719b209df66411107083aafcb5293fc9e: MarkdownBody.tsx shares one SpecSessionPool across its Markdown fences, and SpecView.tsx acquires a session by source text and subscribes to that session's frame/store. There is one P2 in the active-session handoff branch of products/maka/workbench/app/spec/session.ts:350–357: a second independent fence whose complete JSONL source extends the first fence's source reuses the still-mounted first session. Its input edits and compiled frame consequently overwrite the first block. Details are inline.
I ran the five existing spec.test.mjs tests unchanged against the current catalog/schema/session modules and integrity-verified versions from this commit's lockfile; all five passed. Three additional module tests passed for unrelated/equal active siblings and detached remount state, but failed for concurrent prefix siblings (2 passed, 1 failed). In that failing case the second block inherited private A, editing it changed the first block's answer to edited B, and the first frame changed to Second form. A control that disabled only the active-session prefix handoff made all three pass. That control isolates the cause; it is not a complete remount-preserving fix. Fence identity or an explicit remount handoff needs to distinguish these cases.
This is a focused review of session allocation and its Markdown wiring. I did not complete the 33-file review or run native NSH Desktop/Terminal rendering, layout, accessibility, chart, Host, or full CI acceptance. The new dependency/skill/catalog and remaining rendering behavior still need their full checks.
Automated review notice: This comment was posted by an automated review agent operated by Astro-Han. It is not an independent human review and does not replace one.
| } | ||
| } | ||
| } else if ( | ||
| candidate.refCount === 1 && |
There was a problem hiding this comment.
[P2] Keep concurrent prefix-sharing fences in separate sessions
This branch treats a single attached session as a remount whenever a new source extends its text, but a separate completed spec fence in the same Markdown message can have exactly that relationship. MarkdownBody passes one pool to every fence, so mounting A and then independent B = A's valid JSONL plus a label patch returns the same SpecSession for both while both are still mounted. With the unchanged production session module, B inherited A's entered answer; editing B changed A's answer, and ingesting B changed A's subscribed frame to B's label. The existing sibling test uses different source prefixes and misses this case. Use stable fence identity or an explicit handoff signal so source-prefix similarity alone cannot merge two live blocks, while retaining legitimate streaming remount behavior.
Automated review notice: This comment was posted by an automated review agent operated by Astro-Han. It is not an independent human review and does not replace one.
Render inline `spec` Markdown fences inside conversation flows on both Desktop and Terminal using `@json-render/core`, `@json-render/directives`, and a 63-component native NSH registry (including `nsh/base/plot` charts). Split top-level `spec` fences into stable keyed segments in `MarkdownBody` and `BotReply` so streaming updates keep native inputs, focus, and local component signals mounted in place without sibling block collisions. Keep specs strictly client-only (`Link` and `Markdown` links route through the workbench `openLink` handler; `Image` and `Markdown` images route through `MarkdownImageView`), keep `catalog.ts` out of runtime `App.js`, and ship authoring rules as the standalone bundled `maka-generative-ui` skill. Also wait for the composer input before typing in `pending_composer_selects_slash_skills_with_enter_and_tab` to avoid a headless startup race. Generated-by: Google Antigravity
0804ba7 to
d48e18b
Compare
Astro-Han
left a comment
Astro-Han
left a comment
There was a problem hiding this comment.
Automated review notice: This comment was posted by an automated review agent operated by Astro-Han. It is not an independent human review and does not replace one.
I rechecked the session-isolation fix at d48e18b95f136e542eaa61f0ba6dfdc3e4d4a045 and withdraw the P2 reported in my previous review. SpecSessionPool has been removed. MarkdownBody and BotReply split top-level spec fences into stable slot keys, and each mounted SpecView creates its own SpecSession. Source-prefix similarity can no longer select another live block's session through the old pool.
I ran all five unchanged tests from this commit's spec.test.mjs against its production modules and lockfile-matched dependencies; all five passed. An additional controlled composition test using the actual splitSpecMarkdown and SpecSession modules also passed: two prefix-sharing sibling fences kept distinct sessions, A retained private A and its original frame, and B retained its edited value as its source grew and its closing fence arrived. Replacing only session allocation with the previous production pool made that same test fail: the two blocks shared a session, A's answer became edited B, and A's frame changed to B's label.
The composition test models the keyed allocation read in the production JSX; it does not execute NSH's renderer. I have not independently verified native focus/remount behavior, Desktop/Terminal interactions, all 63 components, the expanded image/Host paths, or the full 35-file change. The author's headless/native results remain the author's evidence. This follow-up clears the specific old session-aliasing finding; it is not full PR acceptance. CI for this head was still running when checked.
Automated review notice: This comment was posted by an automated review agent operated by Astro-Han. It is not an independent human review and does not replace one.
Astro-Han
left a comment
Astro-Han
left a comment
There was a problem hiding this comment.
Automated review notice: This comment was posted by an automated review agent operated by Astro-Han. It is not an independent human review and does not replace one.
I reviewed the five-file Table-sorting increment from d48e18b95f136e542eaa61f0ba6dfdc3e4d4a045 to 0e9548708fa89d464564713dc85e8e9dbda6aba6. There is one P2 in products/maka/workbench/app/spec/sort.ts:135–140: selecting numeric-unit comparison only for matching pairs, but natural-text comparison for other pairs, makes the comparator non-transitive. A supported Table column containing 1 s, 900 ms, and 2 forms a three-way ordering cycle. Sorting the same rows from all six input permutations produces three different orders. The inline comment gives the exact comparisons.
All six current spec.test.mjs tests passed unchanged against the production modules and lockfile-matched dependencies. My additional consistency test failed with the actual compareTableCells module. The same six permutations produced one consistent order with the extracted previous production sorting callback, and with a control that selected natural comparison for the whole mixed-domain column. The controls establish the pair-dependent comparison problem; they are not a complete sorting-policy implementation.
The session allocator and its SpecView, MarkdownBody, and BotReply wiring are byte-for-byte unchanged from the previous fix, so the old session-aliasing P2 remains withdrawn. This review is limited to the sorting increment and that source comparison. I did not run NSH's native renderer, focus/remount or Desktop/Terminal interaction tests, or complete acceptance of the full 36-file PR and its image/Host paths. CI for this head was still running when checked.
Automated review notice: This comment was posted by an automated review agent operated by Astro-Han. It is not an independent human review and does not replace one.
| const dir = asc ? 1 : -1; | ||
| const an = parseSortableNumber(a); | ||
| const bn = parseSortableNumber(b); | ||
| if (an !== null && bn !== null && an.domain === bn.domain) { |
There was a problem hiding this comment.
[P2] Use one consistent ordering across the entire Table column
Table rows are converted to strings without requiring a shared unit domain. For a column containing 1 s, 900 ms, and 2, the production comparator returns positive values for all three comparisons: compareTableCells("1 s", "900 ms") === 100, compareTableCells("900 ms", "2") === 898, and compareTableCells("2", "1 s") === 1. The first pair uses converted duration values while the other pairs use natural text, creating a cycle. With the actual module, the six input permutations of these same rows produce three different ascending orders, so a row refresh can change the displayed order even though the sort direction and values are unchanged. Choose a comparison policy once for the column, or a consistent domain ordering, rather than switching policies for each pair; add a mixed-domain transitivity/permutation regression.
Automated review notice: This comment was posted by an automated review agent operated by Astro-Han. It is not an independent human review and does not replace one.
English
Summary
Add native inline Generative UI rendering for
```specMarkdown code blocks inside Maka conversation flows on both Desktop (GPU) and Terminal (TUI), powered by@json-render/core,@json-render/directives, and a 63-component native NSH registry.MarkdownBodyandBotReplysplit top-levelspecfences viasplitSpecMarkdowninto stable slot-keyed segments while preserving canonicalsourceIdbyte offsets on leading Markdown text. EachSpecViewstays mounted in place across streaming chunks so active<Input>focus, uncontrolled local component signals (Show spec source,Accordion,Tabs,Carousel), and user-modified$statesurvive progressive patches—even when multiple siblingspecblocks share overlapping JSONL prefixes in the same message.nsh/base/plotcharts: Covers shadcn + ink layout/display/navigation/overlay/form controls (Card,Stack,Grid,Tabs,Accordion,Dialog,Drawer,Tablewith natural text & unit-domain numeric sorting across duration/byte/currency/percent values and per-columnsortable,Input,NumberInputwith draft-safe clamping,Select,MultiSelect,Slider,ConfirmInput, etc.) plus 7 native charts (LineChart,AreaChart,BarChart,PieChart,ScatterChart,Heatmap,Sparkline). RuntimeApp.jsuses lightweightSPEC_COMPONENT_NAMESsocatalog.tsandschema.tsstay out of the runtime bundle.setState,pushState,removeState,toggleState,resetState,validateForm,copyText),watchcycle protection (MAX_WATCH_CASCADE = 16), directives ($cond,$template,$format,$math,$concat,$count,$truncate,$pluralize,$join), and computed helpers (sum,avg,min,max,count,join).LinkandMarkdownlinks route through the workbenchopenLinkhandler (guarded by row ownership), andImage/Markdownimages route throughMarkdownImageView(host.markdownImageSource+host.readFile).skills/maka-generative-ui/SKILL.mdverified againstbuildSpecCatalogPrompt(), and waits forlabel:New chatbefore typing inpending_composer_selects_slash_skills_with_enter_and_tabto eliminate a headless startup race.Verification
node --test products/maka/workbench/tests/*.test.mjs scripts/ci.test.mjs): 182/182 workbench tests pass, covering catalog/registry/names lockstep, zero runtime imports ofcatalog.ts/schema.ts/zod,SKILL.mdprompt & directive integrity,splitSpecMarkdownsibling prefix overlap & nested outer fences,SpecSessionidempotent incremental patch compilation,committedDocimmutability, user-touched$statesurvival alongside/statekey removals and shrunk arrays, andTableunit-domain sorting (ms/s,B/KB/MB/GB/TB, currency/percent isolation).cargo test -q -p maka-workbench --lib spec_view_tests&cargo test -q -p maka-workbench --lib pending_composer_selects_slash_skills_with_enter_and_tab): Passed on bothIdiom::DesktopandIdiom::Terminalat1080x1600viewport—verifying active$bindState<Input>NodeIdfocus & subsequent keystrokes across mid-stream chunks,Sparklinestreaming insertion,Show spec source& uncontrolledAccordionsignal survival, side-by-side prefix-overlapping siblingspecblocks in one message,Tablealphabetical/natural/duration/byte sorting & per-columnsortable: false,NumberInputdraft typing &minclamping,Tabsvalue="default"single-panel rendering, cyclicwatchtermination at cascade cap,LinkopenLinkcallback, duplicate-keyrepeat+pushState+validateForm, and parse-error fallback recovery.cargo test -q -p maka-skills --test integration bundled_generative_ui_skill— passed.cargo run -q -p maka-workbench --example ui_types -- <tmp>+tsc -p <tmp>/tsconfig.json --noEmit— passed.npm --workspace @maka-agent/product run build && npm --workspace @maka-agent/product run typecheck && npm --workspace @maka-agent/product test— 30 plugin bundles + 4 product tests passed; verifiedcatalog.ts/defineCatalogare absent fromconversations/bundle.js.node scripts/rust/notices.mjs check-reviewed,node scripts/rust/dependencies.mjs check,node scripts/asf-license-headers.mjs check,biome check, andrustfmt --edition 2024 --check— all passed.AI use
Select exactly one:
Tool(s) and scope: Google Antigravity authored the spec splitter/session state engine, native NSH renderer/components/charts, bundled
maka-generative-uiskill, and unit/headless regression tests.Checklist
Does this PR entail a change in behavior?
简体中文
Summary
在 Maka 会话消息流中新增对内联
```specMarkdown 代码块的原生 Generative UI 渲染支持，同时覆盖桌面端（GPU）与终端（TUI），基于@json-render/core、@json-render/directives与 63 个原生 NSH 组件实现。MarkdownBody与BotReply通过splitSpecMarkdown将顶层spec围栏拆分为带稳定槽位 key 的分段，同时保留起始 Markdown 段落的sourceId字节偏移语义。流式追加期间SpecView原地保持挂载，<Input>焦点与后续按键输入、组件局部非受控信号（源码展开态、Accordion、Tabs、Carousel）以及用户修改过的$state均不丢失；同一条消息内存在多个前缀重叠的spec块时亦互不抢占。nsh/base/plot图表：完整覆盖 shadcn + ink 的布局、展示、导航、浮层与表单控件（含支持自然排序、跨单位域数值换算排序与列级sortable的Table、支持编辑草稿态失焦钳制的NumberInput等）以及 7 类原生图表（LineChart、AreaChart、BarChart、PieChart、ScatterChart、Heatmap、Sparkline）。运行时App.js仅依赖轻量常量SPEC_COMPONENT_NAMES，将catalog.ts与schema.ts隔离在测试与 prompt 生成侧。setState、pushState、removeState、toggleState、resetState、validateForm、copyText），内置watch级联步数上限保护（MAX_WATCH_CASCADE = 16），支持全部标准动态指令与$computed（含join）。Link与Markdown链接统一经 workbenchopenLink（含行归属校验）打开；Image与Markdown图片统一走MarkdownImageView（host.markdownImageSource+host.readFile）受控管线。skills/maka-generative-ui/SKILL.md并与buildSpecCatalogPrompt()严格同步校验；同时在pending_composer_selects_slash_skills_with_enter_and_tab中补齐wait_for("label:New chat")，消除无头测试启动竞态。Verification
node --test products/maka/workbench/tests/*.test.mjs scripts/ci.test.mjs）：182 项测试全部通过，覆盖组件白名单/注册表/Catalog 一致性、运行时零catalog.ts/schema.ts/zod直接引用、SKILL.md变量与动作契约完整性、splitSpecMarkdown同消息前缀重叠双块与外层围栏嵌套、SpecSession幂等增量编译、committedDoc不可变性、用户态$state保留、/state删键与数组缩容同步，以及Table时间/字节/货币/百分比同域换算排序与异域隔离。cargo test -q -p maka-workbench --lib spec_view_tests与cargo test -q -p maka-workbench --lib pending_composer_selects_slash_skills_with_enter_and_tab）：在1080x1600视口下同时通过Idiom::Desktop与Idiom::Terminal验证——覆盖流式分片追加期间$bindState<Input>原生节点NodeId保活与焦点连续输入、Sparkline动态插入、源码展开态与非受控Accordion局部状态保活、同消息前缀重叠双spec块并存、Table文本/自然/时间/字节换算排序与列级sortable: false、NumberInput草稿输入与min提交钳制、Tabsvalue="default"单次渲染、循环watch级联熔断、LinkopenLink回调、重复 keyrepeat+pushState+validateForm，以及非法 spec 报错回退与恢复。cargo test -q -p maka-skills --test integration bundled_generative_ui_skill— 通过。cargo run -q -p maka-workbench --example ui_types -- <tmp>+tsc -p <tmp>/tsconfig.json --noEmit— 通过。npm --workspace @maka-agent/product run build && npm --workspace @maka-agent/product run typecheck && npm --workspace @maka-agent/product test— 30 个插件包构建及 4 项产品测试通过，确认conversations/bundle.js不含catalog.ts/defineCatalog。node scripts/rust/notices.mjs check-reviewed、node scripts/rust/dependencies.mjs check、node scripts/asf-license-headers.mjs check、biome check、rustfmt --edition 2024 --check— 全部通过。AI use
Select exactly one:
Tool(s) and scope: Google Antigravity 编写了 spec 分段解析与增量状态引擎、NSH 原生渲染器/组件/图表、内置
maka-generative-uiskill 以及单元与无头集成回归测试。Checklist
Does this PR entail a change in behavior?