Restructure data editor messaging with Message Registry by stricklandrbls · Pull Request #1647 · apache/daffodil-vscode

stricklandrbls · 2026-03-26T17:42:55Z

Closes #1659
Closes #1058
Closes #669

Description

Implemented a Message Registry which provides type-safe message content inspection for messages sent between the data editor, dfdl extension, and the Svelte UI.
Restructured some legacy Svelte stores into Svelte v5 runes.
Resolved issues related to message content mismathcing and message content being processed by multiple data editors when the message was intended for a single data editor instance.

Wiki

I have determined that no documentation updates are needed for these changes
I have added the following documentation for these changes

Review Instructions including Screenshots

Developer Perspective Reviews

In any portion of the data editor's extension source, verify that message content is appropriately inspected and the VSCode intellisense engine emits correct types when invoking this.panel.postMessage(...).

Screenshots ( dataEditorClient.ts -> 'fileInfo')

In any portion of the data editor's Svelte UI source, verify that message content is appropriately inspected and the VSCode intellisense engine emits correct types when invoking addListener(...).

Screenshots ( DataEditorLineFeed.svelte -> 'viewportRefresh')

In any portion of the data editor's Svelte UI source, verify that message content is appropriately inspected and the VSCode intellisense engine emits correct types when invoking vscode.postMessage(...).

Screenshots ( App.svelte -> 'requestEditedData')

Functionality Review

Verify that all message traffic behaves as expected. Message content can be identified within the ./src/ext_types/message.ts & ./src/ext_types/messageContent.ts

CoverRyan

CI seems to be failing consistently and it doesn't appear to be an issue with the CI.

stricklandrbls · 2026-04-09T18:35:18Z

CI seems to be failing consistently and it doesn't appear to be an issue with the CI.

#1659 is the issue I created for fixing the CI

shanedell · 2026-04-10T01:37:15Z

@stricklandrbls Will the updates to fix #1659 be added to this PR? My personal input would be, to add those changes here. I would rather have additional changes in this PR and a working CI then to merge this PR and introduce broken CI into main which requires another PR to fix.

stricklandrbls · 2026-04-10T04:30:12Z

@stricklandrbls Will the updates to fix #1659 be added to this PR? My personal input would be, to add those changes here. I would rather have additional changes in this PR and a working CI then to merge this PR and introduce broken CI into main which requires another PR to fix.

Yeah those changes would fix the CI I created the separate issue because it required multiple additional dev dependencies.

I can add that issue as one that will close this PR.

shanedell · 2026-04-24T17:23:01Z

So I would check if there are two different versions of sax-js in the dependencies.

@shanedell - Wouldn't the latest version's license supersede previous versions?

@stricklandrbls Possibly, I am thinking that yarn lets you pin multiple versions but that might not be right. So the package name is not sax-js you are right there, its actually name sax. But the repo name is sax-js and that might be where that came from. But from looking at yarn.lock it has sax@^1.2.4 and is pinning the version to 1.6.0, so you should only need to list sax once with the BlueOak license

Copilot

Pull request overview

Note

Copilot was unable to run its full agentic suite in this review.

This PR introduces a type-safe “Message Registry” for data editor messaging across the extension and Svelte UI, while migrating parts of the UI state to Svelte v5 runes and tightening message routing to the correct editor instance.

Changes:

Added ext_types message/type definitions and wired them into both extension and Svelte UI message flows.
Replaced legacy window.addEventListener('message', ...) handlers with typed postMessage/addListener helpers plus per-editor message IDs.
Switched some legacy Svelte stores to Svelte v5 rune-based state and updated tests/tooling to use Vitest.

Reviewed changes

Copilot reviewed 50 out of 55 changed files in this pull request and generated 5 comments.

Show a summary per file

File	Description
vite.config.mjs	Adds `ext_types` alias for build/tooling resolution.
tsconfig.json	Adds `ext_types` path mapping for TS type-safe imports.
src/tests/suite/dataEditor.test.ts	Updates extension tests to use new `DataEditorClient` API surface.
src/svelte/vitest.config.mjs	Adds Vitest config for Svelte unit tests/typecheck.
src/svelte/vite.config.mjs	Refactors Svelte Vite config; adds aliasing/testing config.
src/svelte/vite-env.d.ts	Adds extra type references for the Svelte build.
src/svelte/tsconfig.json	Aligns Svelte TS config with repo root and new path aliases.
src/svelte/tests/utilities/display.test.ts	Migrates test runner from Mocha to Vitest and updates types import.
src/svelte/tests/stores/index.test.ts	Removes legacy store tests (replaced by rune-state tests).
src/svelte/tests/stores/fileMetricsState.svelte.test.ts	Adds rune-state tests for file metrics derived state.
src/svelte/svelte.d.ts	Adds reference to data editor global types.
src/svelte/svelte.config.mjs	Adjusts compiler options related to runes/rootDir.
src/svelte/src/utilities/vscode.ts	Introduces typed messenger wrapper with per-editor message IDs.
src/svelte/src/utilities/messages.ts	Dispatches extension responses into typed CustomEvents.
src/svelte/src/utilities/message.ts	Moves shared formatting/types into `ext_types`-compatible exports.
src/svelte/src/utilities/display.ts	Updates imports to use `ext_types` shared types.
src/svelte/src/stores/index.ts	Rewires derived logic to new rune-based `fileMetricsState` helpers.
src/svelte/src/stores/configuration.ts	Switches shared enums/types to `ext_types` and keeps local constants.
src/svelte/src/main.ts	Registers message dispatcher early in UI bootstrap.
src/svelte/src/global.d.ts	Adds UI message-id helpers (currently as runtime code).
src/svelte/src/components/dataEditor.svelte	Removes legacy monolithic component in favor of new structure.
src/svelte/src/components/ServerMetrics/ServerMetrics.svelte	Migrates heartbeat handling to typed `addListener`.
src/svelte/src/components/Inputs/Buttons/Button.svelte	Renames disabled prop to `isDisabled`.
src/svelte/src/components/Header/fieldsets/Settings.svelte	Migrates fileInfo handling to typed `addListener`; adds debug vars.
src/svelte/src/components/Header/fieldsets/SearchReplace.ts	Types `updateSearchResults` against `SearchResponse`.
src/svelte/src/components/Header/fieldsets/SearchReplace.svelte	Migrates search/replace messaging to typed messenger API.
src/svelte/src/components/Header/fieldsets/FileMetrics.ts	Replaces store class with rune-based `fileMetricsState` + derived helpers.
src/svelte/src/components/Header/fieldsets/FileMetrics.svelte	Migrates file metrics + save/undo/redo interactions to typed listeners.
src/svelte/src/components/Header/Header.svelte	Reads filename from rune-based `fileMetricsState`.
src/svelte/src/components/Editors/DataEditor.svelte	Updates edit-mode logic to use `isRegularSizedFile()` helper.
src/svelte/src/components/DataMetrics/DataMetrics.svelte	Migrates profiling messaging and local state naming.
src/svelte/src/components/DataDisplays/Fieldsets/ContentControls.svelte	Updates button API and `EditByteModes` import.
src/svelte/src/components/DataDisplays/CustomByteDisplay/SelectedByteEdit.svelte	Adjusts edit input handling and updates edited byte segment behavior.
src/svelte/src/components/DataDisplays/CustomByteDisplay/DataLineFeed.svelte	Migrates viewport refresh + debug bytePos handling to typed listeners.
src/svelte/src/App.svelte	Centralizes message wiring with per-editor messenger id and typed listeners.
src/svelte/index.html	Injects `extension_msg_id` attribute for per-editor routing.
src/svelte/global.d.ts	Adds global typing for editor message listeners.
src/ext_types/messages.ts	Adds canonical typed request/response payload definitions.
src/ext_types/messageIds.ts	Defines allowed request/response message IDs + type guards.
src/ext_types/messageContent.ts	Defines MessageRequest/Response maps and postMessage arg typing.
src/ext_types/index.ts	Re-exports ext_types and defines listener helper types.
src/dataEditor/ui/svelteWebviewInitializer.ts	New initializer that injects per-editor UI msg id + CSP nonce.
src/dataEditor/ui/displayState.ts	Introduces `DisplayState` for UI-related attributes (encoding/theme/title).
src/dataEditor/svelteWebviewInitializer.ts	Removes legacy initializer.
src/dataEditor/include/utils.ts	Refines DFDL debug session detection and adds `toEncoding`.
src/dataEditor/include/server/heartbeat/index.ts	Updates heartbeat polling to include server info.
src/dataEditor/include/server/heartbeat/HeartBeatInfo.ts	Refactors heartbeat container to include serverInfo.
src/dataEditor/dataEditorClient.ts	Reworks messaging to typed registry; integrates new UI initializer + routing.
src/dataEditor/config/index.ts	Fixes exports to re-export types as types.
package.json	Switches Svelte tests to Vitest; bumps build/test dependencies.
build/package/NOTICE	Adds notices for bundled `lightningcss` and `sax`.
build/package/LICENSE	Adds license text/attribution for bundled dependencies.
build/license_data.json	Adds BlueOak-1.0.0 to allowed licenses list.
.vscode/settings.json	Adds Vitest root config setting and formatting cleanup.

Comments suppressed due to low confidence (4)

src/svelte/vitest.config.mjs:1

With root set to the Svelte workspace directory, tsconfig: './src/svelte/tsconfig.json' and include: ['./src/svelte/tests/...'] will resolve to src/svelte/src/svelte/... and likely fail to find the TS config/tests. Update these paths to be relative to this config’s root (e.g. ./tsconfig.json and ./tests/**/*.svelte.test.ts).
src/svelte/vite.config.mjs:1
loadEnvFile is called but its import was removed in this diff, which will cause the Vite config to crash at runtime. Re-introduce the import (e.g. import { loadEnvFile } from 'node:process') or replace this with Vite’s supported env loading mechanism.
src/svelte/src/utilities/vscode.ts:1
WindowEventMap[K] only works if K is a key of WindowEventMap (built-in DOM events). Here K is a custom message id like 'heartbeat' | 'search' | ...', so this is expected to fail typechecking unless you also augment WindowEventMap with those keys. Consider typing the callback as Event/CustomEvent and casting winEvent to CustomEvent<{ id: string; data: ... }> (or add a proper declare global { interface WindowEventMap { ... } } mapping for each message id).
src/svelte/src/components/ServerMetrics/ServerMetrics.svelte:1
There is a stray serverInfo statement (no-op) and heartbeat.availableProcessors is assigned twice from two different sources (serverCpuCount and serverInfo.availableProcessors), making it unclear which value is authoritative. Remove the no-op line and choose a single source for available processor count (or use distinct fields if both are meaningful).

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

stricklandrbls · 2026-04-24T19:50:41Z

@scholarsmate @shanedell @hdalsania @CoverRyan - I believe this PR is in order and is ready for a re-review. Minor changes were made from the Copilot suggestions.

This PR resolved the CI failing jobs.

Copilot

Pull request overview

Copilot reviewed 50 out of 55 changed files in this pull request and generated 13 comments.

Comments suppressed due to low confidence (1)

src/svelte/src/components/ServerMetrics/ServerMetrics.svelte:31

heartbeat is initialized with omegaEditPort, but later code assigns/uses heartbeat.port (e.g., in the heartbeat listener and template). This adds a new property at runtime and leaves the declared omegaEditPort unused/incorrect. Rename the state field to match the payload (port) or keep using omegaEditPort consistently.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot

Pull request overview

Copilot reviewed 50 out of 55 changed files in this pull request and generated 10 comments.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot

Pull request overview

Copilot reviewed 54 out of 59 changed files in this pull request and generated 8 comments.

Comments suppressed due to low confidence (1)

src/svelte/src/stores/index.ts:265

These derived(...) stores call isRegularSizedFile(), but isRegularSizedFile is backed by a Svelte v5 $derived rune (not a Svelte store). Because it isn’t listed in the dependency array, changes to fileMetricsState.computedSize won’t trigger recomputation, so editMode can become stale when file size changes (e.g. on initial counts message). Consider reintroducing a Svelte-store regularSizedFile dependency, or migrate these derived stores to runes so reactivity stays consistent.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-04-29T12:50:24Z

+  import { getUIMsgId } from 'stores/states.svelte'
  import FlexContainer from '../layouts/FlexContainer.svelte'
+  import { vscode } from 'utilities/vscode'
+  const { addListener } = vscode.getMessenger(getUIMsgId())



vscode.getMessenger(getUIMsgId()) is evaluated at module init time, before setUIMsgId(...) runs in App.svelte. That means this listener will likely be registered with an empty/placeholder id and will never receive events once the real id is set (because _addListener filters by id). Defer creating the messenger until the msg id is initialized (e.g. in onMount / a reactive statement) or make the messenger read the current id at dispatch time.

Copilot · 2026-04-29T12:50:25Z

+  import { getUIMsgId } from 'stores/states.svelte'
+  import { vscode } from 'utilities/vscode'
+  import {
+    canRedo,
+    canRevert,
+    canUndo,
+    fileMetricsState,
+    saveable,
+  } from './FileMetrics.svelte.ts'
+
  const eventDispatcher = createEventDispatcher()

+  const { postMessage, addListener } = vscode.getMessenger(getUIMsgId())
+


vscode.getMessenger(getUIMsgId()) is called at module init time, so it will likely capture uiMsgId === '' (since setUIMsgId runs later in App.svelte). As a result these addListener(...) handlers may never fire because the dispatched events are filtered by id. Create the messenger after the id is known (e.g. in onMount) or refactor the messenger to fetch the current id when handling events.

Copilot · 2026-04-29T12:50:25Z

  getHeartbeatIntervalId =
    activeSessions.length > 0
      ? setInterval(async () => {
-          heartbeatInfo = await getServerHeartbeat(
+          const heartbeat = await getServerHeartbeat(
            activeSessions,
            HEARTBEAT_INTERVAL_MS * activeSessions.length
          )
+          const info = await getServerInfo()
+          heartbeatInfo.serverHeartbeat = {
+            ...heartbeat,
+            serverInfo: { ...info },
+          }
        })


setInterval is missing the interval duration argument here. As written, Node will treat the delay as 0ms/undefined and the server heartbeat polling will run in a tight loop, which can peg CPU and overload the Ωedit server. Pass an explicit delay (likely HEARTBEAT_INTERVAL_MS * activeSessions.length) as the second argument to setInterval.

Copilot · 2026-04-29T12:50:25Z

      id={actionElements['input'].id}
      class:invalid
      class:inProgress
      style:width={elementDivWidth}


In the EditActionRestrictions.None branch the <input> no longer binds to $editorSelection, but the validity/in-progress logic above is based on $editorSelection.length, and the parent handleEditorEvent path relies on $editorSelection to request encoded data. Without updating the store as the user types, the UI state and outgoing messages will be based on stale data. Bind the input value back to the store (or update the store in on:input) so downstream logic stays consistent.

Suggested change

style:width={elementDivWidth}

style:width={elementDivWidth}

bind:value={$editorSelection}

Copilot · 2026-04-29T12:50:26Z

+function isEditorMessage(msg: any): msg is IncomingMessage {
+  return msg && isEditorMessageId(msg.command)
+}
+function isEditorResponse(msg: any): msg is IncomingMessage {
+  return msg && isEditorResponseId(msg.command)
+}


isEditorMessage is declared but never used. With noUnusedLocals enabled in the TS config, this will fail typechecking/build. Remove it or use it (e.g. to validate/handle incoming request messages if intended).

Copilot · 2026-04-29T12:50:26Z

  import Tooltip from '../layouts/Tooltip.svelte'
+  import { getUIMsgId } from 'stores/states.svelte'
+  import { vscode } from 'utilities/vscode'
+  import { fileMetricsState } from 'editor_components/Header/fieldsets/FileMetrics.svelte.ts'


fileMetricsState is imported here but never referenced in the component. With noUnusedLocals enabled, this unused import will break the typecheck/build. Remove the import or use it (e.g. to clamp startOffset/length against current file size if that was the intention).

Suggested change

import { fileMetricsState } from 'editor_components/Header/fieldsets/FileMetrics.svelte.ts'

Copilot · 2026-04-29T12:50:26Z

+          const characterCount = await countCharacters(
+            this.omegaSessionId,
+            startOffset,
+            length
+          ).catch((err) => {
+            console.debug(err)
+          })
+          const contentTypeResponse = await getContentType(
+            this.omegaSessionId,
+            startOffset,
+            length
+          )
+          const languageResponse = await getLanguage(
+            this.omegaSessionId,
+            startOffset,
+            length,
+            characterCount!.getByteOrderMark()
+          )


countCharacters(...).catch(...) can return undefined, but the rest of this handler assumes characterCount is always present (via characterCount!). If countCharacters fails, this will throw when calling getByteOrderMark() and break profiling. Either rethrow/return early on error, or provide a safe fallback BOM / characterCount object before using it.

Copilot · 2026-04-29T12:50:26Z

+export interface MessageResponseMap extends CommandMap {
+  clearChanges: void
+  applyChanges: ChangesInfoResponse
+  editorOnChange: EditorOnChangeResponse
+  fileInfo: FileInfoResponse
+  counts: CountResponse
+  profile: ProfileResponse
+  redoChange: void
+  replaceResults: ReplaceResponse
+  requestEditedData: EditedDataResponse
+  save: SaveResponse
+  saveAs: SaveAsResponse
+  saveSegment: void
+  scrollViewport: void
+  search: SearchResponse
+  replace: ReplaceResponse
+  undoChange: void
+  viewportRefresh: ViewportRefreshResponse
+  showMessage: void
+  setUITheme: SetUIThemeResponse


MessageResponseMap uses void for several no-payload responses (e.g. clearChanges, redoChange, etc.), but PostMessageArgs only treats never as “no payload”. This makes one-arg sends like panel.postMessage('clearChanges') inconsistent with the typing and forces unsafe casts/destructuring. Consider using never for no-payload response entries or updating PostMessageArgs to also treat void | undefined as no-payload.

…ocha

stricklandrbls added this to the 1.6.0 milestone Mar 26, 2026

stricklandrbls requested review from CoverRyan, JeremyYao, duboisfordwork, hdalsania, lrbarber, naga-panchumarty, rthomas320, scholarsmate and shanedell March 26, 2026 17:42

stricklandrbls self-assigned this Mar 26, 2026

stricklandrbls added typescript code quality Issues related to code quality data editor Issues related to the Data Editor capability extension Issues releated to the VSCode Extension labels Mar 26, 2026

stricklandrbls added this to daffodil-vscode v1.6.0 Mar 26, 2026

stricklandrbls force-pushed the data-editor-message-registry branch 3 times, most recently from 65b3fd8 to 8404e0c Compare March 26, 2026 18:11

stricklandrbls moved this to In Review in daffodil-vscode v1.6.0 Mar 27, 2026

stricklandrbls force-pushed the data-editor-message-registry branch 2 times, most recently from dc8d15a to 55d963f Compare March 31, 2026 01:40

CoverRyan previously requested changes Apr 9, 2026

View reviewed changes

stricklandrbls force-pushed the data-editor-message-registry branch 4 times, most recently from e5d837d to d3e7b54 Compare April 13, 2026 21:42

scholarsmate requested review from Copilot and removed request for duboisfordwork April 24, 2026 18:49

Copilot AI reviewed Apr 24, 2026

View reviewed changes

Copilot started reviewing on behalf of scholarsmate April 24, 2026 19:03 View session

stricklandrbls force-pushed the data-editor-message-registry branch from e0430b7 to d312dbd Compare April 24, 2026 19:49

stricklandrbls force-pushed the data-editor-message-registry branch 2 times, most recently from 8647ef3 to 9f2630f Compare April 24, 2026 19:56

scholarsmate requested a review from Copilot April 25, 2026 02:15

Copilot started reviewing on behalf of scholarsmate April 25, 2026 02:16 View session

Copilot AI reviewed Apr 25, 2026

View reviewed changes

stricklandrbls force-pushed the data-editor-message-registry branch 5 times, most recently from 72c22db to ed1adec Compare April 25, 2026 06:44

scholarsmate requested a review from Copilot April 26, 2026 14:39

Copilot started reviewing on behalf of scholarsmate April 26, 2026 14:40 View session

Copilot AI reviewed Apr 26, 2026

View reviewed changes

stricklandrbls force-pushed the data-editor-message-registry branch 4 times, most recently from e290c32 to c59b811 Compare April 29, 2026 03:49

scholarsmate requested a review from Copilot April 29, 2026 12:39

Copilot started reviewing on behalf of scholarsmate April 29, 2026 12:40 View session

Copilot AI reviewed Apr 29, 2026

View reviewed changes

Implemented Data Editor Message Registry w/ svelte.ts tests fix for M…

c54d287

…ocha

stricklandrbls force-pushed the data-editor-message-registry branch from c59b811 to c54d287 Compare April 29, 2026 16:18

	style:width={elementDivWidth}
	style:width={elementDivWidth}
	bind:value={$editorSelection}

Conversation

stricklandrbls commented Mar 26, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Wiki

Review Instructions including Screenshots

Developer Perspective Reviews

Functionality Review

Uh oh!

CoverRyan left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

stricklandrbls commented Apr 9, 2026

Uh oh!

shanedell commented Apr 10, 2026

Uh oh!

stricklandrbls commented Apr 10, 2026

Uh oh!

shanedell commented Apr 24, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

stricklandrbls commented Apr 24, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Copilot AI Apr 29, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 29, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 29, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 29, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 29, 2026

Choose a reason for hiding this comment

stricklandrbls commented Mar 26, 2026 •

edited

Loading

CoverRyan left a comment •

edited

Loading

shanedell commented Apr 24, 2026 •

edited

Loading