Skip to content

refactor: one copy of each toolkit helper - #122

Open
endrix wants to merge 2 commits into
mainfrom
refactor/toolkit-helpers
Open

endrix wants to merge 2 commits into
mainfrom
refactor/toolkit-helpers

Conversation

@endrix

@endrix endrix commented Nov 8, 2026 •

Copy link
Copy Markdown
Owner

Phase 4b of the declutter plan, first half: the duplicated helpers inside the toolkit and the URI key spelled in three packages. The second half (one sidecar transport, one export-op default) follows as its own PR.

What was duplicated

helper copies before now
fileExists, findPackageRootForFile source-analysis, create-node-handler, sidecar-graph-export, cli-run-driver (fs.stat variant) server/source-files.ts
PYTHONPATH augmentation from the package root invokeSidecarOp, exportWorkflowGraph, tryLoadGraphFromSidecar withPythonPackageRoot
collectPythonFilesUnderRoots source-analysis (ceilings, dot-dirs skipped), create-node-handler (own exclusion set, no ceilings, sorted) one function with an options object; each caller keeps its own rule
countParenDelta source-analysis, python-diagram-definitions, python-text (string-blanked variant) python-text.ts exports the raw one; its own blanked variant is countParenDeltaOutsideStrings
escapeRegExp source-analysis, python-text, python-navigation (escapedRegexSymbol) python-text.ts
stripPythonCommentsAndStrings source-analysis, python-text python-text.ts
unqualifyName two handlers, equal to normalizeWorkflowReferenceName the latter
CLI command resolution cli-run-driver resolveCliInvocation re-implemented getCliInvocation + the Python-interpreter regex getCliInvocation, whose config parameter is now the four CLI fields (CliCommandConfig)
normalizeSourceUriKey extension-core, sidecar-toolkit (two: vscode.Uri+normalize, vscode-uri+resolve), diagram-server, diagram-client (two, URL-based) @dialogram/diagram-server/server/uri-keys for host, server and toolkit; diagram-client/src/source-uri-key.ts for the webview

source-analysis.ts re-exports what it used to define, so its importers and tests are unchanged.

Behaviour, and what the review made of it

Meant to be none. Three edges the review sharpened:

  • The run driver keeps its guard against a blank CLI command (a profile may set cliCommandDefault: ''), and its error names what is actually missing: the command, or a root (workspace folder, package.json or .git). A blank setting now falls back to the default, as it does for every other reader.
  • The one normalizeSourceUriKey resolves the path (path.resolve), as the server's copy did, so a trailing slash or a . segment makes no second key; the host's copy normalized only.
  • withPythonPackageRoot always returns a copy, never the live process.env.

Also from the review: the webview had a third copy of the name-splitting rule, so unqualifiedName now lives in @dialogram/shared and both the property panel and normalizeWorkflowReferenceName use it; source-analysis.ts re-exports nothing it does not define (the caller scan's bounded file list is callerScanSourceFiles, its importers read uri-keys, python-text and source-files directly); the scan ceiling's warning takes its hint from the caller; the stale comment in graph-load-request-options.ts is gone.

Tests

source-files.test.ts (package root, PYTHONPATH, scan options and ceiling), uri-keys.test.ts. Typecheck 5/5, neutrality 5/5, server 443, client 361, extension-core 331, toolkit 330 with the known local wizard timeouts (green when the two files run alone, green in CI).

Cut from main before #119–#121; only CHANGELOG.md can conflict.

https://claude.ai/code/session_01EToj6RJRMSSHQiqGNMU9wv

Phase 4b, first half. The toolkit carried the same small helpers in
several files, each a verbatim or near copy: fileExists and
findPackageRootForFile in four, the PYTHONPATH augmentation in three,
collectPythonFilesUnderRoots in two, countParenDelta in three,
escapeRegExp in three, stripPythonCommentsAndStrings in two,
unqualifyName in two beside normalizeWorkflowReferenceName, and a CLI
command resolution in the run driver beside the server's.

server/source-files.ts holds the file-system helpers, python-text.ts
the text helpers, and the rest import them. The run driver resolves the
CLI command through getCliInvocation; a blank setting now means the
default, as it does everywhere else.

normalizeSourceUriKey had four copies in three packages and a parity
test to keep two of them aligned. One function, in
diagram-server/server/uri-keys.ts, serves the host, the server and the
toolkit; the webview keeps one browser-bound copy of its own instead of
two.

Claude-Session: https://claude.ai/code/session_01EToj6RJRMSSHQiqGNMU9wv
The run driver keeps its guard against a blank CLI command and says
what is missing when it cannot run; the one source-URI key resolves the
path, as the server's did, so a trailing slash makes no second key; the
scan ceiling's warning says what the caller wants said; a package-root
environment is always a copy; the name rule the webview and the toolkit
both apply is one function in shared, unqualifiedName; and
source-analysis re-exports nothing it does not define: the caller
scan's file list is callerScanSourceFiles, the rest is imported from
where it lives.

Claude-Session: https://claude.ai/code/session_01EToj6RJRMSSHQiqGNMU9wv
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant