feat(core): flat models[] config + internal field migration (PR 1 of 3)#776
Open
loopingz wants to merge 13 commits into
Open
feat(core): flat models[] config + internal field migration (PR 1 of 3)#776loopingz wants to merge 13 commits into
loopingz wants to merge 13 commits into
Conversation
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
- New `models: string[]` parameter - `model` and `additionalModels` mapped to `models[]` with WARN log - Throws if both old and new shapes are set - Deprecation tests cover the five compat cases
…ll comment Code review follow-ups for Task 1: - Conflict between models[] and additionalModels detected even when additionalModels is empty (presence check, not length) - Backfill comment explains why ??= is a no-op in the legacy branch - Two new tests: empty-additionalModels conflict and additionalModels-only fallback to Webda/RegistryEntry Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
- Drop _model, _modelMetadata, _modelType from Store class - Add _models: ModelClass[] (index 0 = legacy primary) - Add _modelMetadatas: Map<string, ModelMetadata> - computeParameters() walks parameters.models[] (canonical list from Task 1) - recursive() normalises ModelClass refs to string IDs as _modelsHierarchy keys - getModel() returns _models[0] (deprecated, kept for back-compat) - getModels() added as new public method - setModelDefinitionHelper() sets _models[0] for test-harness compat - Two new @test methods verify _models/_modelMetadatas and getModel() back-compat Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
- populatesModelsArrayAndMetadatas now uses models: [...] constructor arg and sets getParameters().models directly before resolve() to exercise computeParameters() via the canonical models[] path (not the legacy shim) - Removed dead-code test stubs from memory.spec.ts (file is excluded from vitest) - Dropped extra useLog TRACE noise from computeParameters() Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
- walksSubclassHierarchyForNonStrictStore exercises recursive() ModelClass branch via Webda/User → Webda/SimpleUser depth-1 traversal - strictStoreSkipsSubclassWalk verifies strict-mode short-circuit - populatesModelsArrayAndMetadatas now also asserts getModels().length Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Replaces dropped _modelType field. In strict mode, accept files whose __type matches a depth-0 model in this store's models[]; subclasses (depth > 0) remain excluded. Two new tests cover accept and reject paths. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…ters.table for multi-model
- One model in models[]: parameters.table maps to that model (compat shortcut)
- Multiple models: parameters.table is ignored with WARN log at init()
- parameters.tables[id] continues as the canonical per-model override
- Default derivation (id.lower.replace('/', '_')) unchanged
- 3 new tests cover single, multi, and explicit-map paths in a separate
PostgresStoreResolveTableTest suite (no DB connection required)
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
- fs/filestore.ts:205: this._model → this._models[0] - fs/filestore-unit.spec.ts: getWithStrictMode now sets _modelsHierarchy directly (matching what computeParameters does at runtime) - runtime/invitationservice.spec.ts: this.store._model = X → setModelDefinitionHelper(X) Closes the migration started in Task 2. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
After Tasks 1-5, StoreParameters exposes a flat models: string[] alongside deprecated model + additionalModels. Regenerated schemas accept both shapes. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This reverts commit 84ac94d.
…eld" This reverts commit ce6f411.
…els[] field"" This reverts commit 1f57114.
Owner
Author
Task 7 (schema regen) — investigation updateAfter ~30 min of debugging the regression, found that running The schema diff itself looks superficially benign:
Things ruled out:
Best guess: when both legacy For now, Task 7 is reverted via |
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
1 participant
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
First of three PRs for the Repository concept refactor. This PR introduces the flat
models: string[]parameter onStoreParameters, migratesStoreinternals from_model/_modelMetadata/_modelTypeto_models[]/_modelMetadatasMap, and updates downstream stores (fs,postgres,runtime) to match.Spec:
docs/superpowers/specs/2026-05-09-repository-design.md(local-only — gitignored).Plan:
docs/superpowers/plans/2026-05-09-repository-design.md(local-only — gitignored).What's in this PR (Tasks 1–5 of 8)
StoreParametersacceptsmodels: string[]alongside legacymodel+additionalModels; deprecationWARNlog fires on legacy use; ambiguity guard throws when both shapes are set.Storeclass migrated to_models: ModelClass[]and_modelMetadatas: Map<string, ModelMetadata>._modelTypedropped. NewgetModels()accessor;getModel()deprecated.computeParameters()rewritten to walkparameters.models[]. Latent bug fixed inrecursive()subclass walker (the runtime type fromApplication.setModelMetadatais(string | ModelClass)[], notstring[]as the type declared).FileStore._get()strict-mode__typecheck rewritten to use_modelsHierarchy(replaces dropped_modelTypereference).PostgresStore.resolveTable()migrated off_modelMetadata;parameters.tablenow treated as a single-model shortcut, withWARNlog when set with multiple models._model/_modelTypereferences infs/filestore.ts:205,fs/filestore-unit.spec.ts:299,runtime/invitationservice.spec.ts:100migrated.What's not in this PR (deferred)
model:tomodels:) — cosmetic; deprecation shim keeps existing configs working.webda.module.jsonschemas) — surfaced a load-bearing regression where the legacymodel: { default: "Webda/CoreModel" }schema default is essential for framework-loaded stores to claim every model. Reverted in commitce6f4112. Needs a follow-up architectural fix.PRs 2 (Repository event re-emission) and 3 (API positioning / docs) follow this one.
Backward compatibility
{ model: "X", additionalModels: [...] }configs keep working via the deprecation shim. AWARNlog fires once per service noting the migration path.getModel()still returns the first model for back-compat (marked@deprecated).useStore()is unchanged in this PR.Test plan
pnpm --filter @webda/core test— 554 passed, 1 todo (was 550 before this PR; 4 new tests added).pnpm --filter @webda/fs test— 123 passed (was 122; 1 new strict-mode test).pnpm --filter @webda/postgres test— 3 newresolveTabletests pass; pre-existing PostgresStoreSmokeTest failures unrelated (DB connectivity).How this was built
Each task: TDD (failing test → implementation → passing test → commit). Two-stage subagent review per task (spec compliance, then code quality), with fix loops where reviewers found issues. Two follow-up commits for code-review feedback land on top of the original task commits where applicable.
🤖 Generated with Claude Code