fix(metadata): non-mutating getters, ingestion crash fixes, faster levels/description handling#134
Merged
Merged
Conversation
… & description memoization
Getter cluster (A): getMetadata/getMetadataFields assemble on a fresh object
instead of mutating the live private metadata (author/variableMeasured no longer
leak into internal state); VariablesMap.getList and collapseDescription operate
on copies so a second generate() after getMetadata() can't corrupt the stored
per-plugin description maps. collapseDescription now lets a user-edited "default"
win (leads the joined Text) and only drops it when it is the "unknown"
placeholder — the frontend's user-edited descriptions survive.
Ingestion crashes (B): extension_type/extension_version normalized to arrays
(JSON-array strings from CSV are parsed, single strings wrapped, missing values
tolerated) so generate() no longer crashes on `.map`/undefined indexing; string
cells are only coerced to numbers when they round-trip (String(Number(v))===v),
keeping "007", 17-digit ids and "1e5" as string levels; non-object observations
(a null row) are skipped with a warning. updateDescription accepts a plain
string as { default: text } (no more char-by-char spread) and the empty-object
guard now compares to real undefined, not the string "undefined". updateMinMax
initializes only the missing bound; updateName refuses collisions/empty names;
updateLevels stringifies primitive levels.
Perf (C): per-(variable, plugin) description fetch+merge is memoized per
generate() call; levels dedup is O(1) via an internal Set while the serialized
field stays an insertion-ordered string[]; the extension loop no longer
re-runs updateFields on a value already folded in; opt-in levelsCap (default
off). Also default the verbose constructor param and fix the displayMetadata
JSDoc. Existing tests that pinned the old destructive-getter / dropped-default /
raw-numeric-level behavior are updated to the corrected behavior.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01QMjKvi9vHT3LfikGfW84Kg
- objectsToCSV now escapes header names (a column key with a comma or quote no
longer produces a structurally corrupt sidecar CSV).
- parseJsonData filters null observations out of its JSONL output so a `null`
line can't emit [null] and crash downstream Object.keys(null).
- Remove the unused JSON2CSV helper (superseded by objectsToCSV).
- Update the CSV stress test: scientific-notation strings ("1e3") now stay as
string levels under the round-trip coercion rule instead of being coerced to
numeric ranges.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01QMjKvi9vHT3LfikGfW84Kg
Keying the plugin-description cache on plugin name alone served the first-seen version's descriptions to every later version in a mixed-version dataset, and made a plugin and an extension that share a name collide on one entry. Key on (isExtension, name, version) so each is cached independently. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01QMjKvi9vHT3LfikGfW84Kg
Adds packages/metadata/tests/correctness-perf-fixes.test.ts (extension column normalization, plain-string descriptions, user-edited default survival, non-mutating getMetadata interleaved with generate, null observation handling, objectsToCSV header escaping, numeric round-trip, updateMinMax/updateName/ updateLevels guards, version-keyed PluginCache, and description-fetch memoization) and a frontend integration test that drives the REAL @jspsych/ metadata library the way Variables.tsx does and renders the real PreviewDrawer, proving an edited description reaches the output JSON. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01QMjKvi9vHT3LfikGfW84Kg
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01QMjKvi9vHT3LfikGfW84Kg
🦋 Changeset detectedLatest commit: 2b83d04 The changes in this PR will be included in the next version bump. This PR includes changesets to release 2 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
…y-string description defaults
Two review follow-ups:
- The strict round-trip numeric test reclassified non-canonical decimals
("450.0", "5.50", "1.0") as categorical string levels — silently
de-numericizing whole float columns from R/pandas exports. Accept
decimal-fraction literals (no leading zeros) alongside the round-trip;
identifier protection is unaffected since those risks ("007", 17-digit
ids, "1e5") are all integer/exponent-shaped.
- collapseDescription treated a cleared (empty-string) user default as real
user text, producing a leading " | " before plugin descriptions. Treat
it like the "unknown" placeholder.
Tests for both, changeset updated to document the exact rules.
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
2 participants
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
Implements the core-library workstream of the repository improvement plan (
docs/dev/improvement-plan.md, Phases 1a/1b/2). Every bug below was verified by executing a reproduction before fixing; every fix ships with the regression test that would have caught it.The destructive-getter cluster (silent loss of user-edited descriptions)
collapseDescriptionnow lets a user-editeddefaultdescription win; it is only dropped when it is the"unknown"placeholder. Previously, editing a description for any plugin-documented or system variable in the web wizard showed the edit in the UI but shipped the plugin text in the downloaded JSON.getList()collapses descriptions on copies — it no longer mutates stored variables, sogetMetadata()followed by anothergenerate()(the CLI's multi-file flow) no longer corrupts/duplicates description text.getMetadata()/getMetadataFields()assemble and return fresh copies;author/variableMeasuredno longer leak into internal state.Ingestion crashes / corruption (all previously reproduce as thrown errors or corrupt output)
generate()for CSV input ('["mouse-tracking"]'string), missingextension_version, or single-stringextension_type.descriptionin metadata options (generate(data, { variables: { rt: { description: "Reaction time" } } })) is treated as{ default: text }instead of being spread character-by-character into{"0":"R","1":"e",…}.nullrows in JSON/JSONL input are skipped with a warning instead of crashing.objectsToCSVescapes header names (a column key containing a comma previously produced a structurally corrupt sidecar CSV).String(Number(v)) === v.trim()), so"007", 17+-digit IDs, and"1e5"stay strings and the metadata no longer disagrees with the verbatim-written CSV.updateMinMaxno longer clobbers a single pre-set bound;updateNamerefuses renames onto existing/empty names instead of silently deleting variables;updateLevelsstringifies primitives.PluginCacheis keyed on (name, version, extension flag) so mixed-version datasets get the right descriptions.Performance (no default output changes)
Set— wasArray.includesper value, O(n²); a 94k-distinct-value column was ~4×10⁹ comparisons. New opt-inlevelsCapgenerate option (default off; the no-cap default behavior is unchanged and still covered by the scale stress test).(plugin, version, variable)pair pergenerate()call — previously every cell awaited the plugin cache and re-merged.JSON2CSV, fixed stale JSDoc, defaultedconstructor(verbose = false).Testing
tests/correctness-perf-fixes.test.ts(20 tests) covering every fix, plus a frontend integration test (packages/frontend/tests/descriptionSurvival.test.tsx) that drives the real library the wayVariables.tsxdoes and asserts an edited description reaches the output JSON.#getListnon-mutation, two/three-key default-description survival, numeric-level stringification, scientific-notation strings staying strings.tsc --noEmitclean; package build succeeds.Notes for review
getVariable/getVariableListstill return live references by design —updateFieldsand the CLI's description-prompt flow depend on that aliasing; changing it is deferred (see improvement plan Phase 1a caution).🤖 Generated with Claude Code
https://claude.ai/code/session_01QMjKvi9vHT3LfikGfW84Kg
Generated by Claude Code