refactor: consolidate type guards, validate JSON boundaries with zod, remove duplicated micro-helpers#303
Merged
Merged
Conversation
…nd test helpers Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Follow-up to #302. That PR removed the hand-rolled
isRecordguards; this one finishes the job (the extension-side commit missed the merge — cherry-picked here as the first commit) and cleans up the ten related code smells found in a monorepo-wide sweep.Progress update
✅ Items 1–2: consolidated object-record probes and JSON-state parsing; switched repo-settings clone to
structuredClone.✅ Item 3: kept host/webview equality comparators separate after documenting their
undefinedkey semantics; shared the record guard plumbing.✅ Item 4: consolidated finite-number, positive-number, and pair guards in graph physics numeric helpers.
✅ Item 5: moved string-array/string readers onto shared schema-backed value helpers while preserving boundary-specific string semantics.
✅ Item 6: replaced plugin-runtime message probing with zod schemas that keep lenient filtering behavior.
✅ Item 7: schema-validated the remaining unvalidated persisted JSON boundaries; left already-normalized parse sites unchanged.
✅ Item 8: covered by the repo-settings clone update in items 1–2.
✅ Item 9: centralized unavoidable Tree-sitter language casts in one helper path.
✅ Item 10: intentionally kept core and extension
unknownRecordSchemapackage-local to avoid expanding core's public API for a generic zod helper.Plan of action
0. Leftover
isRecordfrom #302 ✅ (first commit, cherry-picked)e2e/workspacePlugins.ts,pluginData.ts,cssSnippets.ts, webviewguards.ts/linkTargets.ts, acceptance helpers — zod at JSON boundaries, direct property checks for live d3-force objects.1–2.
isPlainObject/isGraphRecordclones (4 copies)repoSettings/store/model/plainObject.ts,webview/store/messageHandlers/equality/structures.ts,webview/components/legends/panel/storage.ts,graphView/analysis/execution/publish/equality/collections.ts.Approach: where the value comes from JSON/persisted state, parse with the shared
unknownRecordSchema; where it's a structural probe in equality code, keep ONE shared typed helper and delete the clones.3. Duplicate deep-equality implementations
Extension-host
publish/equality/*vs webviewmessageHandlers/equality/structures.ts. Approach: compare semantics; if identical, extract one comparator intosrc/shared/; if intentionally different (host compares graph payloads, webview compares message state), document why and still deduplicate the record/array plumbing. Conservative: no behavior change, existing tests must stay green.4.
isFiniteNumber× 4+ webview copies (+isFinitePositiveNumber,isFinitePair)Consolidate into one exported helper next to the existing
runtime/physics/numeric.ts, update all import sites, delete clones.5.
readStringArray/readOptionalString/readRequiredStringre-implementationsrepoSettings/store/model/persistedShape/stringArray.ts, e2e + acceptance helpers, coregraphCache/database/records/values.ts. Approach: move to the shared zod value schemas (looseStringArraySchemaetc.) introduced in #302; core keeps its own copy incore/src/values.ts, extension usessrc/shared/values.ts.6. Webview plugin-runtime message validation
webview/pluginRuntime/messageValidation.tshand-probespayload as {...}field by field. Replace with zod schemas producingPluginInjectPayload/PluginWebviewAsset/PluginScopedMessage, preserving current lenient semantics (filter bad entries, don't throw).7. Unvalidated
JSON.parse+ cast boundaries (audit + schema-fy)repoSettings/store/persistence/diskState.ts,gitHistory/cache/storage.ts,graphView/groups/defaults/materialTheme/manifest.ts,repoSettings/meta.ts, coreplugins/installedPluginCache/storage.ts,workspace/meta.ts. Approach: audit each; where the parsed value is cast or property-probed, add a zod schema; where it's already routed through a normalizer, leave it.8.
deepCloneviaJSON.parse(JSON.stringify(...))repoSettings/store/model/plainObject.ts— switch tostructuredClone(Node 20+ / modern webview), verify no reliance on JSON's undefined-dropping behavior in call sites first.9. 19×
as unknown as Parser.Languageincore/treeSitter/runtime/languages/load.tsExtract a single typed
loadLanguageModule()helper so the unavoidable cast appears once, documented.10. Duplicate
unknownRecordSchema(core + extension)Decide single source: keep both (packages are published independently) OR export from
@codegraphy-dev/core. Default plan: export from core's public API and delete the extension copy, unless bundle/API-surface concerns say otherwise.Method
🤖 Generated with Claude Code