feat(lifecycle): persist raw declarations and recompute effective models - #245
Merged
Conversation
- Persist module models to meta_raw_* only and recompute effective projections without parent-chain materialize. - Retarget uninstall, FieldDefault/AppSetting supersede, i18n ensure, and schema DDL reads to raw, then recompute; remove tip rebind. Co-authored-by: Cursor <cursoragent@cursor.com>
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughThe PR moves metadata flows to raw declarations plus recomputed effective projections. It updates build, lifecycle, i18n, schema, and runtime consumers to load raw records, expand inheritance, and recompute effective metadata. It also updates tests and helper APIs for the dual-store model. ChangesDual-store metadata flow
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant ModuleBuilder
participant RawMetadata
participant RecomputeKeys
participant ExpandModelsAlongExtends
participant EffectiveMetadata
ModuleBuilder->>RawMetadata: Persist raw model trees
ModuleBuilder->>RecomputeKeys: Recompute logical keys
RecomputeKeys->>RawMetadata: Load raw trees
RecomputeKeys->>ExpandModelsAlongExtends: Expand inherited fields and services
ExpandModelsAlongExtends->>RawMetadata: Load parent models
RecomputeKeys->>EffectiveMetadata: Persist effective projection
Possibly related PRs
Suggested labels: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
Comment |
PR Reviewer Guide 🔍Here are some key observations to aid the review process:
|
PR Code Suggestions ✨No code suggestions found for the PR. |
Codecov Report❌ Patch coverage is 📢 Thoughts on this report? Let us know! |
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
- Mint raw tree IDs and expand Extends before schema migrate / E2 recompute so thin subclasses keep inherited columns and services. - Omit Models on install/upgrade Module.Save and clear in-memory trees after persist so declaration shells no longer duplicate effective rows. - Read codegen models by application name and prefer empty-module_id rows in UI rpc dependency checks. Co-authored-by: Cursor <cursoragent@cursor.com>
- Reuse tip service ids by method name when rewriting the effective subtree so Auth ACL rows keyed by meta_service_id stay valid. - Stop deleting legacy module_id effective rows before recompute so EDS5 can keep the tip model id. Co-authored-by: Cursor <cursoragent@cursor.com>
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 6
🧹 Nitpick comments (9)
internal/module/evolution/schema/migrator_test.go (1)
105-116: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winCover inheritance expansion in the schema-loading test.
The fixture contains only standalone raw models. It verifies raw preloads and filtering, but not the new
RawModelsAsModelsplusExpandModelsAlongExtendspath. Add an abstract base model with a field and a child model withExtends, then assert that the loaded child contains the inherited field.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@internal/module/evolution/schema/migrator_test.go` around lines 105 - 116, The schema-loading fixture in the test should cover inheritance expansion, not only standalone models. Extend the rawModels setup with an abstract base model containing a field and a child model referencing it through Extends, then assert after loading and expansion that the child includes the inherited field while preserving existing filtering assertions.internal/module/artifact/generate/generator_test.go (1)
348-387: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winExercise the effective projection path in the canonical-model test.
All models in this fixture have a valid
ModuleId. The test can pass with the old module join. Add a separate model withApplication: "crm"and an emptyModuleId, then assert thatgetApplication()loads it.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@internal/module/artifact/generate/generator_test.go` around lines 348 - 387, Extend the canonical-model fixture near the existing model variables with a separate model whose Application is “crm” and ModuleId is empty, then include it in the persisted test data. Update the canonical-model assertions to call getApplication() for this model and verify it is loaded, exercising the effective projection path independently of the module join.pkg/meta/dual_store_migrate_coverage_test.go (1)
595-600: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd coverage for the non-nil reuse map.
Every
persistEffectiveProjectioncall in this file passesnilforreuseServiceIDs, so the reuse branch and the!usedServiceIDs[prev]duplicate guard indual_store_migrate.golines 544-549 stay uncovered.TestRecomputeEffective_PreservesServiceIDsByNameexercises the happy path throughRecomputeEffective, but not the case where two merged services share a trimmed name and only one can take the prior id.Add one direct case here with a reuse map and two same-named services. Assert that the second service receives a fresh id.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@pkg/meta/dual_store_migrate_coverage_test.go` around lines 595 - 600, Extend the direct persistEffectiveProjection coverage with a non-nil reuseServiceIDs map and two merged services sharing the same trimmed name; verify the first service reuses the prior ID and the second receives a fresh ID, covering the !usedServiceIDs[prev] duplicate guard without changing existing failure assertions.pkg/meta/extends_expand_test.go (1)
65-85: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAssert the no-write invariant.
ExpandModelsAlongExtendsdocuments that it does not write to the database, andcloneFieldShallow/cloneServiceShallowclearBaseModeland foreign keys to keep that true. The test does not verify it. Add row counts onModel,Field, andServiceafter expansion, and assert the cloned entities carry empty ids.♻️ Proposed additional assertions
if !svcNames["Create"] || !svcNames["Normalize"] { t.Fatalf("unexpected services: %#v", svcNames) } + for _, f := range child.Fields { + if f.Id.Valid && f.Id.String != "" { + t.Fatalf("expanded field must stay unpersisted: %#v", f) + } + } + for _, tbl := range []any{&Model{}, &Field{}, &Service{}} { + var n int64 + if err := db.Model(tbl).Count(&n).Error; err != nil { + t.Fatalf("count: %v", err) + } + if n != 0 { + t.Fatalf("expansion must not write effective rows, got %d", n) + } + } }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@pkg/meta/extends_expand_test.go` around lines 65 - 85, Add no-write invariant checks to the test around ExpandModelsAlongExtends: capture Model, Field, and Service row counts before expansion and assert they are unchanged afterward, then verify expanded cloned fields and services have empty IDs. Keep the existing field and service name assertions intact.pkg/meta/dual_store_migrate.go (2)
536-550: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winDocument why prior service-id reuse cannot collide.
The reuse branch assigns a prior effective service id to a newly created row. This is only safe because
RecomputeEffectivecallsDeleteEffectiveModelTreefor every existing row before it callspersistEffectiveProjection, so the ids are free at insert time. A future caller that passes a non-nilreuseServiceIDswithout deleting first would hit a primary-key conflict.Add that precondition to the doc comment on
persistEffectiveProjectionand on thereuseServiceIDsparameter.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@pkg/meta/dual_store_migrate.go` around lines 536 - 550, Update the documentation for persistEffectiveProjection and its reuseServiceIDs parameter to state that reuseServiceIDs may only be provided after existing effective rows have been deleted, as performed by RecomputeEffective via DeleteEffectiveModelTree before persistEffectiveProjection; otherwise reused IDs can cause primary-key conflicts.
283-345: 🚀 Performance & Scalability | 🔵 Trivial | ⚖️ Poor tradeoffConsider batching the raw-tree inserts.
copyModelTreeToRawissues oneCreateper field, service, parameter, type parameter, decorator, and argument. A module with several hundred models produces thousands of round trips on every build and on every IMD migration. TheensureBaseModelIDchange means every row now has its id assigned in Go before insert, so the rows can be collected per entity type and written withCreateInBatches.This is a performance improvement only; the current code is correct. Defer it if build time is acceptable today.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@pkg/meta/dual_store_migrate.go` around lines 283 - 345, Defer this optional performance improvement unless build time requires it; if implemented, update copyModelTreeToRaw to collect each raw entity type, including fields, services, parameters, type parameters, decorators, and arguments, after ensureBaseModelID, then persist each collection with GORM CreateInBatches while preserving existing relationships and error propagation.pkg/meta/recompute.go (1)
67-71: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winRemove the no-op probe and stop discarding the lock error.
Line 68 runs
SELECT 1and discards the result. It does not serialize anything, and the comment above it suggests otherwise. Delete it.
lockLogicalKeyalso discards the error from theFOR UPDATEquery (line 204) and always returns nil.gorm.ErrRecordNotFoundis expected when no effective row exists yet, but a lock timeout, serialization failure, or connection error is silently ignored, and the recompute continues without the lock it believes it holds. Return the non-ErrRecordNotFounderrors.♻️ Proposed cleanup
- // Serialize concurrent recomputes for the same logical name (best-effort on SQLite). - _ = tx.Exec("SELECT 1").Error + // Serialize concurrent recomputes for the same logical name (best-effort on SQLite). if err := lockLogicalKey(tx, key); err != nil { return err }case "postgres": // Advisory lock keyed by app+name hash would be ideal; fall back to FOR UPDATE // on any existing effective row (no-op when missing). var m Model - _ = tx.Clauses(clause.Locking{Strength: "UPDATE"}). + err := tx.Clauses(clause.Locking{Strength: "UPDATE"}). Where("application = ? AND name = ?", key.Application, key.Name). Take(&m).Error + if err != nil && !errors.Is(err, gorm.ErrRecordNotFound) { + return fmt.Errorf("lock effective %s/%s: %w", key.Application, key.Name, err) + }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@pkg/meta/recompute.go` around lines 67 - 71, Remove the discarded SELECT 1 probe before lockLogicalKey in the recompute flow. Update lockLogicalKey to propagate errors from its FOR UPDATE query while treating gorm.ErrRecordNotFound as the expected no-row case; return all other errors so recomputation does not continue without a valid lock.internal/module/metaeff/recompute.go (1)
12-23: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueThe wrapper is bypassed by at least one consumer.
This package forwards to
pkg/metawithout adding behavior.internal/i18n/models/i18n_meta.goline 58 callsmeta.RecomputeEffectivedirectly instead of going throughmetaeff. Two entry points for the same operation make it harder to add incremental-recompute policy later, which the package doc states is its purpose.Pick one path. Either route the internal callers through
metaeff, or drop the package until it holds real logic.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@internal/module/metaeff/recompute.go` around lines 12 - 23, Ensure all internal callers use a single recomputation entry point: update the direct meta.RecomputeEffective call in i18n metadata handling to go through metaeff.RecomputeEffective, or remove the pass-through metaeff package and its wrappers. Preserve the existing recomputation behavior while eliminating duplicate access paths.pkg/meta/recompute_test.go (1)
202-229: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winThe assertion does not prove deduplication.
RecomputeEffectiveis idempotent, so recomputing{a, X}once or twice both leave exactly one row.count == 1therefore holds whether or notRecomputeKeysdedupes. The invalid key{Application: " ", Name: "X"}also produces no row on its own, so it is not observed either.To test the actual behavior, count the recompute invocations, or assert that the effective row's
UpdatedAt/id is untouched by a second pass. Alternatively, assertLogicalKey{Application: " ", Name: "X"}.Valid()is false directly to cover the skip branch.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@pkg/meta/recompute_test.go` around lines 202 - 229, Strengthen TestRecomputeKeys_Dedupes so it observes deduplication rather than only the idempotent final row count: either instrument and assert that the recompute path runs once for duplicate valid keys, or capture the effective row’s identity/timestamp and verify the duplicate pass leaves it unchanged. Also add a direct assertion that LogicalKey{Application: " ", Name: "X"}.Valid() is false to cover the invalid-key skip branch.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@internal/i18n/models/i18n_meta.go`:
- Around line 71-73: Update the raw I18n lookup to select by the canonical path
and application rather than logical name, while preserving the existing ordering
behavior. Add a regression test that creates multiple raw I18n declarations for
the same application and verifies recomputation updates or creates the canonical
go://i18n/<application> declaration instead of modifying an extension.
In `@internal/module/artifact/build/backend/builder.go`:
- Around line 330-353: Update the raw extends query in the lookup block before
`extendsRaws` is populated to filter by both application and model name, using
`model.Application` and falling back to `b.module.ApplicationStr` when the model
application is unset. Preserve the existing ordering and conversion logic, and
add a test covering same-name declarations from different applications to ensure
the selected extends path remains within the current application.
In `@internal/module/evolution/schema/foreignkey.go`:
- Around line 148-160: Update resolveTargetModelByPath to constrain the RawModel
query by the target module’s ModuleId as well as path, resolving the owning
module before converting the result so duplicate paths across modules cannot
select the wrong ModelTable. Preserve the existing conversion and empty-result
handling, and add a regression test covering identical paths declared in
different modules.
In `@internal/module/evolution/schema/migrator.go`:
- Around line 46-57: Update newMigrator and all callers of getModuleModels to
handle its returned error before invoking Migrate(); propagate the error from
ExpandModelsAlongExtends rather than constructing a migrator with incomplete
models. Ensure both migrators fail early when model loading or inheritance
expansion fails.
In `@internal/module/metaeff/recompute_test.go`:
- Around line 96-99: The fixture cleanup deletes ignore errors in both recompute
test files. In internal/module/metaeff/recompute_test.go at lines 96-99, check
and fail on errors from every RawField delete at lines 99, 118, and 134,
matching the adjacent RawModel deletes; in pkg/meta/recompute_test.go at lines
103-106, do the same for RawField deletes at lines 106 and 125. Use the existing
test failure pattern so cleanup failures are reported immediately.
In `@pkg/meta/recompute_test.go`:
- Around line 192-199: The loop iterating over second.Services will not execute
if the collection is empty, causing the test to pass without verifying any
service properties. Add an assertion that validates the service count before the
loop over second.Services to ensure the test actually performs the id rotation
check rather than passing vacuously when no services exist.
---
Nitpick comments:
In `@internal/module/artifact/generate/generator_test.go`:
- Around line 348-387: Extend the canonical-model fixture near the existing
model variables with a separate model whose Application is “crm” and ModuleId is
empty, then include it in the persisted test data. Update the canonical-model
assertions to call getApplication() for this model and verify it is loaded,
exercising the effective projection path independently of the module join.
In `@internal/module/evolution/schema/migrator_test.go`:
- Around line 105-116: The schema-loading fixture in the test should cover
inheritance expansion, not only standalone models. Extend the rawModels setup
with an abstract base model containing a field and a child model referencing it
through Extends, then assert after loading and expansion that the child includes
the inherited field while preserving existing filtering assertions.
In `@internal/module/metaeff/recompute.go`:
- Around line 12-23: Ensure all internal callers use a single recomputation
entry point: update the direct meta.RecomputeEffective call in i18n metadata
handling to go through metaeff.RecomputeEffective, or remove the pass-through
metaeff package and its wrappers. Preserve the existing recomputation behavior
while eliminating duplicate access paths.
In `@pkg/meta/dual_store_migrate_coverage_test.go`:
- Around line 595-600: Extend the direct persistEffectiveProjection coverage
with a non-nil reuseServiceIDs map and two merged services sharing the same
trimmed name; verify the first service reuses the prior ID and the second
receives a fresh ID, covering the !usedServiceIDs[prev] duplicate guard without
changing existing failure assertions.
In `@pkg/meta/dual_store_migrate.go`:
- Around line 536-550: Update the documentation for persistEffectiveProjection
and its reuseServiceIDs parameter to state that reuseServiceIDs may only be
provided after existing effective rows have been deleted, as performed by
RecomputeEffective via DeleteEffectiveModelTree before
persistEffectiveProjection; otherwise reused IDs can cause primary-key
conflicts.
- Around line 283-345: Defer this optional performance improvement unless build
time requires it; if implemented, update copyModelTreeToRaw to collect each raw
entity type, including fields, services, parameters, type parameters,
decorators, and arguments, after ensureBaseModelID, then persist each collection
with GORM CreateInBatches while preserving existing relationships and error
propagation.
In `@pkg/meta/extends_expand_test.go`:
- Around line 65-85: Add no-write invariant checks to the test around
ExpandModelsAlongExtends: capture Model, Field, and Service row counts before
expansion and assert they are unchanged afterward, then verify expanded cloned
fields and services have empty IDs. Keep the existing field and service name
assertions intact.
In `@pkg/meta/recompute_test.go`:
- Around line 202-229: Strengthen TestRecomputeKeys_Dedupes so it observes
deduplication rather than only the idempotent final row count: either instrument
and assert that the recompute path runs once for duplicate valid keys, or
capture the effective row’s identity/timestamp and verify the duplicate pass
leaves it unchanged. Also add a direct assertion that LogicalKey{Application: "
", Name: "X"}.Valid() is false to cover the invalid-key skip branch.
In `@pkg/meta/recompute.go`:
- Around line 67-71: Remove the discarded SELECT 1 probe before lockLogicalKey
in the recompute flow. Update lockLogicalKey to propagate errors from its FOR
UPDATE query while treating gorm.ErrRecordNotFound as the expected no-row case;
return all other errors so recomputation does not continue without a valid lock.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 885bf1bc-d9b1-4729-8ebb-6bb057a2b8ab
📒 Files selected for processing (34)
internal/i18n/models/i18n_meta.gointernal/i18n/models/i18n_meta_test.gointernal/module/artifact/build/backend/app_setting.gointernal/module/artifact/build/backend/app_setting_coverage_test.gointernal/module/artifact/build/backend/app_setting_test.gointernal/module/artifact/build/backend/builder.gointernal/module/artifact/build/backend/builder_test.gointernal/module/artifact/build/backend/field_default.gointernal/module/artifact/build/backend/field_default_coverage_test.gointernal/module/artifact/build/backend/field_default_test.gointernal/module/artifact/build/web/webBuilder.gointernal/module/artifact/generate/generator.gointernal/module/artifact/generate/generator_test.gointernal/module/evolution/schema/foreignkey.gointernal/module/evolution/schema/foreignkey_test.gointernal/module/evolution/schema/helpers_test.gointernal/module/evolution/schema/migrator.gointernal/module/evolution/schema/migrator_test.gointernal/module/lifecycle/bundles_app_setting_test.gointernal/module/lifecycle/bundles_field_default_test.gointernal/module/lifecycle/installer.gointernal/module/lifecycle/uninstaller.gointernal/module/lifecycle/uninstaller_clean_models_test.gointernal/module/lifecycle/uninstaller_model_data_test.gointernal/module/lifecycle/upgrader.gointernal/module/metaeff/recompute.gointernal/module/metaeff/recompute_test.gopkg/meta/dual_store_migrate.gopkg/meta/dual_store_migrate_coverage_test.gopkg/meta/dual_store_migrate_test.gopkg/meta/extends_expand.gopkg/meta/extends_expand_test.gopkg/meta/recompute.gopkg/meta/recompute_test.go
- Add extends_expand and recompute coverage suites, including delete/merge/lock hooks for late error paths. - Cover i18n/builder/web/schema/lifecycle/app-setting/field-default patch gaps and remove unreachable RawModelsAsModels empty checks. Co-authored-by: Cursor <cursoragent@cursor.com>
- Select the canonical go://i18n path when seeding I18n raw services. - Scope extends lookup by application and reject ambiguous FK target tables. - Propagate schema model-load errors from NewMigrator and lockLogicalKey. Co-authored-by: Cursor <cursoragent@cursor.com>
Contributor
Author
|
@cursor review |
- Exercise FK effective/raw lookup errors and ambiguous tables. - Hit NewMigrator failures in install/upgrade and lockLogicalKey errors. Co-authored-by: Cursor <cursoragent@cursor.com>
- Promote the canonical I18n raw UpdatedAt past same-name extensions before recompute. - Assert effective Path and built-in services stay on go://i18n/<application>. Co-authored-by: Cursor <cursoragent@cursor.com>
- Force promoteI18nCanonicalTip failure after a successful seed so the EnsureI18nMeta error return is exercised. Co-authored-by: Cursor <cursoragent@cursor.com>
Contributor
Author
|
@cursor review |
- Promote created_at with updated_at so pickTipRaw and E2 tip agree on go://i18n. - Assert the first effective model id matches the canonical raw id. Co-authored-by: Cursor <cursoragent@cursor.com>
Contributor
Author
|
@cursor review |
There was a problem hiding this comment.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
internal/module/artifact/build/backend/builder_test.go (1)
712-740: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick winAssert the effective projection after raw replacement.
Line 712 seeds a stale
meta.Model, but the test only checksmeta.RawModelafterpersistModuleModels. A regression that leaves/models/staleinmeta_modelwill pass. Querymeta.Modeland assert that only the currentpartnerprojections remain.Proposed test assertion
if len(persisted) != 2 || persisted[0].Path != "/models/order" || persisted[1].Name != "Partner" { t.Fatalf("unexpected persisted raw models: %#v", persisted) } + + var effective []*meta.Model + if err := db.Where("application = ?", "partner").Order("path ASC").Find(&effective).Error; err != nil { + t.Fatalf("query effective models: %v", err) + } + if len(effective) != 2 || effective[0].Path != "/models/order" || effective[1].Path != "/models/partner" || effective[1].Name != "Partner" { + t.Fatalf("unexpected effective models: %#v", effective) + }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@internal/module/artifact/build/backend/builder_test.go` around lines 712 - 740, Extend the persistModuleModels test after the existing RawModel assertions to query meta.Model for module-1 and assert that only the current partner projections remain, excluding the seeded stale model and nil entry. Keep the assertion focused on the expected model count and paths/names so regressions leaving /models/stale in meta_model fail.internal/i18n/models/i18n_meta.go (1)
51-64: 🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy liftSerialize canonical raw-model initialization and recomputation.
EnsureI18nMetadoes not protect raw-model creation, service registration, tip promotion, recomputation, and ACL seeding as one operation. The(path, module_id)unique index permits duplicate canonical rows whenmodule_idis NULL or differs. Serialize the full(application, i18nModelName)operation and add a concurrent regression test.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@internal/i18n/models/i18n_meta.go` around lines 51 - 64, Update EnsureI18nMeta to serialize the entire (application, i18nModelName) workflow, covering ensureI18nRawModel, ensureI18nRawServices, promoteI18nCanonicalTip, meta.RecomputeEffective, and ACL seeding under one per-key lock or equivalent. Preserve existing error propagation while preventing concurrent initialization from creating duplicate canonical rows, and add a regression test that runs concurrent initialization for the same application and verifies only one canonical raw model exists.
🧹 Nitpick comments (2)
pkg/meta/recompute_coverage_test.go (1)
525-530: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winRemove the unreachable branch.
tc.dropholds a*RawModelpointer for the first case.&RawModel{}allocates a new pointer, so the comparison is always true. The else branch never runs, and both branches drop the same table. The comment about drop ordering describes behavior that the code does not implement.♻️ Proposed simplification
- if tc.drop != (&RawModel{}) { - _ = db2.Migrator().DropTable(tc.drop) - } else { - // Drop after pluck would need model to exist first; drop before second call. - _ = db2.Migrator().DropTable(&RawModel{}) - } + // For the RawModel case this drops the table before the pluck, which + // makes the first load fail. + _ = db2.Migrator().DropTable(tc.drop)🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@pkg/meta/recompute_coverage_test.go` around lines 525 - 530, Remove the unreachable else branch in the test cleanup around tc.drop, and always call db2.Migrator().DropTable(tc.drop). Delete the obsolete drop-ordering comment while preserving the existing cleanup behavior.pkg/meta/extends_expand_coverage_test.go (1)
39-90: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueThe test does not prove a cache hit.
The assertions only confirm that both children inherit the parent field and service. They also pass when
expandShapeAlongExtendsreloads the parent from the database for each child. Count the parent loads to assert cache reuse, or rename the test to describe shared-parent inheritance.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@pkg/meta/extends_expand_coverage_test.go` around lines 39 - 90, Update TestExpandModelsAlongExtends_CacheHitViaTwoChildren to verify cache reuse by instrumenting or counting parent loads and asserting the shared parent is loaded only once while expanding both children. Keep the existing inheritance assertions, or rename the test to reflect shared-parent inheritance if load-count instrumentation is not available.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@internal/module/artifact/build/backend/builder_test.go`:
- Around line 1530-1539: Replace the PRAGMA query_only approach with a
connection-independent failure injection by registering a Before callback on the
"gorm:create" hook for the "meta_raw_model" table in the test. Within the
callback, call tx.AddError to inject the failure that matches the expected error
message "persist raw model". Use t.Cleanup to unregister the callback after the
test completes. This ensures the failure injection applies regardless of which
connection from GORM's pool is used for the persistModuleModels insert
operation.
In `@internal/module/artifact/build/web/webBuilder_test.go`:
- Around line 3764-3798: Reorder the fixture in the “prefers effective model
over declaration shell” test so the shell model is persisted before the
effective model. Keep the existing model and service definitions unchanged,
ensuring the test exercises validation when an unordered first-row lookup
returns the shell.
In `@pkg/meta/recompute_coverage_test.go`:
- Around line 765-774: The test manually restores
mergeSameNameModelsByExtensionChainFn at the end, but if t.Fatalf is called
during the test, line 774 never executes and the stub persists for subsequent
tests. Replace the manual restoration at the end with a t.Cleanup call
(registered after saving prevMerge) that restores
mergeSameNameModelsByExtensionChainFn regardless of whether the test passes or
fails, following the same pattern already established for lockLogicalKeyFn and
expandModelsAlongExtendsFn in this test.
- Around line 50-79: Update the namedDialector-based test setup in
TestRecomputeEffective_PostgresLockBranch and
TestLockLogicalKey_PostgresNonNotFoundError so it exercises the PostgreSQL
branch without emitting SQLite-incompatible FOR UPDATE SQL. Preserve the
postgres Name() result while configuring the wrapped SQLite dialector to omit or
neutralize the locking clause, allowing RecomputeEffective to succeed and the
dropped-table case to reach and return the expected “lock effective row” error.
---
Outside diff comments:
In `@internal/i18n/models/i18n_meta.go`:
- Around line 51-64: Update EnsureI18nMeta to serialize the entire (application,
i18nModelName) workflow, covering ensureI18nRawModel, ensureI18nRawServices,
promoteI18nCanonicalTip, meta.RecomputeEffective, and ACL seeding under one
per-key lock or equivalent. Preserve existing error propagation while preventing
concurrent initialization from creating duplicate canonical rows, and add a
regression test that runs concurrent initialization for the same application and
verifies only one canonical raw model exists.
In `@internal/module/artifact/build/backend/builder_test.go`:
- Around line 712-740: Extend the persistModuleModels test after the existing
RawModel assertions to query meta.Model for module-1 and assert that only the
current partner projections remain, excluding the seeded stale model and nil
entry. Keep the assertion focused on the expected model count and paths/names so
regressions leaving /models/stale in meta_model fail.
---
Nitpick comments:
In `@pkg/meta/extends_expand_coverage_test.go`:
- Around line 39-90: Update TestExpandModelsAlongExtends_CacheHitViaTwoChildren
to verify cache reuse by instrumenting or counting parent loads and asserting
the shared parent is loaded only once while expanding both children. Keep the
existing inheritance assertions, or rename the test to reflect shared-parent
inheritance if load-count instrumentation is not available.
In `@pkg/meta/recompute_coverage_test.go`:
- Around line 525-530: Remove the unreachable else branch in the test cleanup
around tc.drop, and always call db2.Migrator().DropTable(tc.drop). Delete the
obsolete drop-ordering comment while preserving the existing cleanup behavior.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 91fecf79-5729-4e7e-a482-e33acabe44fa
📒 Files selected for processing (23)
internal/i18n/models/i18n_meta.gointernal/i18n/models/i18n_meta_test.gointernal/module/artifact/build/backend/app_setting_coverage_test.gointernal/module/artifact/build/backend/builder.gointernal/module/artifact/build/backend/builder_test.gointernal/module/artifact/build/backend/field_default_coverage_test.gointernal/module/artifact/build/web/webBuilder_test.gointernal/module/evolution/schema/foreignkey.gointernal/module/evolution/schema/foreignkey_test.gointernal/module/evolution/schema/migrator.gointernal/module/evolution/schema/migrator_test.gointernal/module/lifecycle/installer.gointernal/module/lifecycle/installer_commit_test.gointernal/module/lifecycle/uninstaller_clean_models_test.gointernal/module/lifecycle/upgrader.gointernal/module/metaeff/recompute_test.gopkg/meta/dual_store_migrate.gopkg/meta/dual_store_migrate_coverage_test.gopkg/meta/extends_expand.gopkg/meta/extends_expand_coverage_test.gopkg/meta/recompute.gopkg/meta/recompute_coverage_test.gopkg/meta/recompute_test.go
💤 Files with no reviewable changes (1)
- pkg/meta/extends_expand.go
🚧 Files skipped from review as they are similar to previous changes (11)
- internal/module/lifecycle/upgrader.go
- internal/module/metaeff/recompute_test.go
- internal/module/lifecycle/installer.go
- pkg/meta/recompute_test.go
- pkg/meta/dual_store_migrate_coverage_test.go
- internal/module/artifact/build/backend/app_setting_coverage_test.go
- pkg/meta/dual_store_migrate.go
- internal/module/lifecycle/uninstaller_clean_models_test.go
- pkg/meta/recompute.go
- internal/module/artifact/build/backend/field_default_coverage_test.go
- internal/module/artifact/build/backend/builder.go
- Prefer same-application raw parents when expanding Extends along path. - Run RecomputeEffective delete+persist in one transaction to avoid catalog holes. - Harden related builder/web/recompute tests for preference and rollback paths. Co-authored-by: Cursor <cursoragent@cursor.com>
Contributor
Author
|
@cursor review |
- Exercise cross-application Extends parent fallback and load errors. - Promote tip via sibling CreatedAt and skip declaration shells after effective. Co-authored-by: Cursor <cursoragent@cursor.com>
There was a problem hiding this comment.
✅ Bugbot reviewed your changes and found no new issues!
Comment @cursor review or bugbot run to trigger another review on this PR
Reviewed by Cursor Bugbot for commit eb9df43. Configure here.
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@pkg/meta/extends_expand_coverage_test.go`:
- Around line 144-163: Update the test fixtures in the parent creation loop for
foreign and home to assign explicit, distinct creation timestamps, making
foreign newer than home. Preserve the existing application, path, and insertion
setup while ensuring the timestamp fields used by recency ordering are populated
deterministically.
- Around line 178-195: Update expandShapeAlongExtends caching to distinguish
parent shapes by both application and model path, or by the resolved parent
identity, so same-path declarations from different applications cannot collide.
Extend the coverage in ExpandModelsAlongExtends to expand home and foreign
children together, asserting each receives only its own application’s parent
fields while retaining its child fields and preserving same-path local
declarations in a batch.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 185996bb-2104-495e-89d2-ecfa6a77c923
📒 Files selected for processing (8)
internal/i18n/models/i18n_meta.gointernal/i18n/models/i18n_meta_test.gointernal/module/artifact/build/backend/builder_test.gointernal/module/artifact/build/web/webBuilder_test.gopkg/meta/extends_expand.gopkg/meta/extends_expand_coverage_test.gopkg/meta/recompute.gopkg/meta/recompute_coverage_test.go
🚧 Files skipped from review as they are similar to previous changes (7)
- pkg/meta/extends_expand.go
- internal/i18n/models/i18n_meta.go
- internal/i18n/models/i18n_meta_test.go
- internal/module/artifact/build/backend/builder_test.go
- internal/module/artifact/build/web/webBuilder_test.go
- pkg/meta/recompute_coverage_test.go
- pkg/meta/recompute.go
- Add declaration helpers so i18n, schema, AppSetting, FieldDefault, and uninstall stop using Raw* CRUD directly. - Keep effective recompute and uninstall cleanup on the facade path, and rename auth rawModel to modelFullName. - Resolve e2e Playwright from workspace node_modules so a PATH Homebrew install cannot skew the test registry. Co-authored-by: Cursor <cursoragent@cursor.com>
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (4)
pkg/meta/declaration_test.go (1)
149-155: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAssert that the child rows are deleted.
The test is named
_Cascadebut only countsRawModelrows. OrphanedRawFieldandRawServicerows would still pass. Add counts for the child tables so a regression in the cascade order is caught.♻️ Proposed additional assertions
var n int64 if err := db.Unscoped().Model(&RawModel{}).Where("id = ?", id).Count(&n).Error; err != nil || n != 0 { t.Fatalf("left=%d err=%v", n, err) } + for name, dest := range map[string]any{"field": &RawField{}, "service": &RawService{}} { + var c int64 + if err := db.Unscoped().Model(dest).Where("model_id = ?", id).Count(&c).Error; err != nil || c != 0 { + t.Fatalf("%s rows left=%d err=%v", name, c, err) + } + }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@pkg/meta/declaration_test.go` around lines 149 - 155, Extend the cascade assertions in the DeleteDeclarationTrees test to count RawField and RawService rows associated with the deleted declaration, in addition to RawModel. Fail the test when either child-table count is nonzero, while preserving the existing error reporting and parent-row assertion.internal/module/lifecycle/uninstaller_model_data_test.go (1)
174-180: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAssert the identity of the surviving effective model.
The test counts one effective
(partner, Partner)row. A stale effective row that recomputation failed to replace also satisfies that count. Load the row and assert itsPathequalsbasePathand itsModuleIdequalsbaseMod.Id. The assertion then proves recomputation reprojected from the surviving base declaration.♻️ Proposed assertion
- var effectiveCount int64 - if err := db.Model(&meta.Model{}).Where("application = ? AND name = ?", "partner", "Partner").Count(&effectiveCount).Error; err != nil { - t.Fatalf("count effective model: %v", err) - } - if effectiveCount != 1 { - t.Fatalf("survivor raw should yield one effective model, count = %d", effectiveCount) - } + var effectives []meta.Model + if err := db.Where("application = ? AND name = ?", "partner", "Partner").Find(&effectives).Error; err != nil { + t.Fatalf("load effective models: %v", err) + } + if len(effectives) != 1 { + t.Fatalf("survivor raw should yield one effective model, count = %d", len(effectives)) + } + if effectives[0].Path != basePath || effectives[0].ModuleId.String != baseMod.Id.String { + t.Fatalf("effective model = {path:%q module:%q}, want {path:%q module:%q}", + effectives[0].Path, effectives[0].ModuleId.String, basePath, baseMod.Id.String) + }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@internal/module/lifecycle/uninstaller_model_data_test.go` around lines 174 - 180, Extend the effective-model verification after the count in the relevant test to load the surviving meta.Model row for application “partner” and name “Partner”, then assert its Path equals basePath and ModuleId equals baseMod.Id. Preserve the existing count assertion and use the loaded row to prove recomputation reprojected the surviving base declaration.pkg/meta/declaration.go (2)
86-130: 🗄️ Data Integrity & Integration | 🔵 Trivial | ⚡ Quick winThe declaration facade mutates the raw catalog without transactions. Both write paths in
pkg/meta/declaration.goissue several dependent statements through the bare*gorm.DBhandle. A failure partway through leaves the raw catalog in a state that no single statement produced: a declaration with a newmodule_idand a partial service set, or child rows deleted while their parent models survive. The shared remediation is to run each sequence insidedb.Transactionand pass the transaction handle to every statement.
pkg/meta/declaration.go#L86-L130: wrap the lookup, themodule_idupdate, and the per-service creates indb.Transaction, and replace thedbreferences in the body with the transaction handle. This also closes the window where two concurrent callers both miss the service existence check and create duplicate rows.pkg/meta/declaration.go#L203-L253: base thefresh()closure on the transaction handle instead ofroot, so the six-step cascade over arguments, decorators, type parameters, parameters, services, fields, and models commits or rolls back as one unit.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@pkg/meta/declaration.go` around lines 86 - 130, Wrap the declaration update flow around PersistModelTreeAsRaw in db.Transaction, using the transaction handle for the lookup, module_id update, and per-service creation. Also update the fresh() cascade at pkg/meta/declaration.go lines 203-253 to use the transaction handle instead of root, ensuring all dependent deletes and writes commit or roll back together; both sites are in the same file.
256-263: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winDerive the raw table names from the models.
HasEffectiveCataloguses(&Model{}).TableName(), butHasDeclarationCataloghardcodes"meta_raw_model"and"meta_raw_service". If a raw model'sTableName()changes, this check returnsfalseand callers such asEnsureI18nMetaskip registration silently and returnnil. Use the same derivation in both helpers.♻️ Proposed change
m := db.Migrator() - return m.HasTable("meta_raw_model") && m.HasTable("meta_raw_service") + return m.HasTable(&RawModel{}) && m.HasTable(&RawService{})🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@pkg/meta/declaration.go` around lines 256 - 263, Update HasDeclarationCatalog to derive both table names from the corresponding model TableName methods, matching HasEffectiveCatalog, instead of hardcoding string literals. Preserve the existing nil-database handling and require both derived tables to exist.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@pkg/meta/declaration.go`:
- Around line 49-61: Add id ASC ordering callbacks to every unordered
association preload in the PreloadTree block of the declaration query, including
Services, Services.Parameters, Services.TypeParameters, Services.Decorators,
Services.Decorators.Arguments, Decorators, and Decorators.Arguments. Preserve
the existing ordering callbacks for Fields and its nested associations.
---
Nitpick comments:
In `@internal/module/lifecycle/uninstaller_model_data_test.go`:
- Around line 174-180: Extend the effective-model verification after the count
in the relevant test to load the surviving meta.Model row for application
“partner” and name “Partner”, then assert its Path equals basePath and ModuleId
equals baseMod.Id. Preserve the existing count assertion and use the loaded row
to prove recomputation reprojected the surviving base declaration.
In `@pkg/meta/declaration_test.go`:
- Around line 149-155: Extend the cascade assertions in the
DeleteDeclarationTrees test to count RawField and RawService rows associated
with the deleted declaration, in addition to RawModel. Fail the test when either
child-table count is nonzero, while preserving the existing error reporting and
parent-row assertion.
In `@pkg/meta/declaration.go`:
- Around line 86-130: Wrap the declaration update flow around
PersistModelTreeAsRaw in db.Transaction, using the transaction handle for the
lookup, module_id update, and per-service creation. Also update the fresh()
cascade at pkg/meta/declaration.go lines 203-253 to use the transaction handle
instead of root, ensuring all dependent deletes and writes commit or roll back
together; both sites are in the same file.
- Around line 256-263: Update HasDeclarationCatalog to derive both table names
from the corresponding model TableName methods, matching HasEffectiveCatalog,
instead of hardcoding string literals. Preserve the existing nil-database
handling and require both derived tables to exist.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 20a3fcbd-816f-4889-8271-ac1317e9eb01
📒 Files selected for processing (30)
internal/i18n/models/i18n_meta.gointernal/i18n/models/i18n_meta_test.gointernal/module/artifact/build/backend/app_setting.gointernal/module/artifact/build/backend/app_setting_coverage_test.gointernal/module/artifact/build/backend/app_setting_test.gointernal/module/artifact/build/backend/builder.gointernal/module/artifact/build/backend/builder_test.gointernal/module/artifact/build/backend/field_default.gointernal/module/artifact/build/backend/field_default_coverage_test.gointernal/module/artifact/build/backend/field_default_test.gointernal/module/evolution/schema/foreignkey.gointernal/module/evolution/schema/foreignkey_test.gointernal/module/evolution/schema/helpers_test.gointernal/module/evolution/schema/migrator.gointernal/module/evolution/schema/migrator_test.gointernal/module/lifecycle/installer_commit_test.gointernal/module/lifecycle/uninstaller.gointernal/module/lifecycle/uninstaller_clean_models_test.gointernal/module/lifecycle/uninstaller_model_data_test.gointernal/module/metaeff/recompute_test.gointernal/testing/e2e/runner.gointernal/testing/e2e/runner_test.gomodules/auth/service/models/_user_field_rule_eval.tsmodules/auth/service/models/user.tsmodules/auth/service/tests/authz_context_memoization.test.tsmodules/auth/service/tests/field_rule.test.tspkg/meta/declaration.gopkg/meta/declaration_test.gopkg/meta/recompute.gopkg/meta/recompute_coverage_test.go
🚧 Files skipped from review as they are similar to previous changes (16)
- internal/module/evolution/schema/foreignkey_test.go
- internal/module/evolution/schema/migrator.go
- internal/module/lifecycle/uninstaller.go
- internal/module/artifact/build/backend/builder.go
- pkg/meta/recompute_coverage_test.go
- internal/module/lifecycle/installer_commit_test.go
- internal/module/artifact/build/backend/builder_test.go
- internal/module/metaeff/recompute_test.go
- internal/module/artifact/build/backend/app_setting_coverage_test.go
- internal/module/artifact/build/backend/field_default_test.go
- pkg/meta/recompute.go
- internal/module/artifact/build/backend/app_setting.go
- internal/module/evolution/schema/foreignkey.go
- internal/i18n/models/i18n_meta_test.go
- internal/module/evolution/schema/migrator_test.go
- internal/module/artifact/build/backend/field_default_coverage_test.go
- Align RunModule hook fixtures with workspace-first playwright resolution so PATH-only stubs no longer fail preflight. Co-authored-by: Cursor <cursoragent@cursor.com>
- Key expand-shape cache/local lookups by application+path so same-path parents from different apps cannot collide, and cover the batch case. - Order declaration PreloadTree associations by id, wrap upsert/delete cascades in transactions, and derive HasDeclarationCatalog table names from models. - Strengthen cascade and uninstall recompute assertions while keeping effective ModuleId omitted by design. Co-authored-by: Cursor <cursoragent@cursor.com>
Contributor
Author
|
@cursor review |
There was a problem hiding this comment.
✅ Bugbot reviewed your changes and found no new issues!
Comment @cursor review or bugbot run to trigger another review on this PR
Reviewed by Cursor Bugbot for commit 6a04215. Configure here.
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.
User description
Summary
meta_raw_*, thenRecomputeKeysrebuilds unique effectivemeta_model*projections (stable ids); parent-chain materialize is no longer written to DB.rebindMetaModelDataTips); FieldDefault/AppSetting supersede and i18n ensure write/delete raw then recompute; schema migrator/FK path resolve read raw by module/path.metaeff; builder/lifecycle/FD·AS/i18n/schema coverage updated for dual-store.Test plan
go test ./pkg/meta/ ./internal/module/metaeff/ ./internal/module/lifecycle/ ./internal/module/artifact/build/backend/ ./internal/i18n/models/ ./internal/module/evolution/schema/ -count=1partner+partner_bank+partner_commercialand confirm one effective Partner with union fields and stable id across uninstall of commercial/bankmeta_raw_*and recomputes effectiveDepends on #244 (EDS-1). Unlocks EDS-3/4 readers + ModelData.
Made with Cursor
PR Type
Enhancement, Tests
Description
Persist raw declarations into meta_raw_* tables
Introduce incremental effective catalog recompute logic
pkg/meta/recompute.gowith LGPL-3.0 SPDX headers.internal/module/metaeff/recompute.gowrapper with LGPL-3.0 SPDX headers.Update module uninstaller and callers for dual-store
Expand Go test coverage across core packages
pkg/meta,metaeff, lifecycle, and schema suites.modules/).File Walkthrough
9 files
Implement catalog recomputation and raw persistence helper methodsExpose incremental recomputation wrapper package with SPDX headersUpdate module builder to persist raw models and recomputeRetarget module uninstaller to raw entities and recompute keysRetarget AppSetting loading and supersede logic to raw entitiesRetarget FieldDefault loading and supersede logic to raw entitiesRegister I18n declarations on raw tables and recompute effectiveFetch raw model declarations for schema migration filteringResolve target foreign key models from raw model declarations10 files
Add Go unit tests for effective catalog recomputationAdd fixture test for partner extension model recomputeAdapt builder backend unit tests for dual-store tablesUpdate cleanModels error branch unit tests for raw tablesUpdate uninstaller tests for raw declaration deletion and recomputeUpdate i18n metadata unit tests for dual-store raw tablesUpdate AppSetting builder tests to seed raw modelsUpdate FieldDefault builder tests to seed raw modelsUpdate schema migrator tests to create raw model fixturesAdd raw field helper functions for schema evolution tests1 files
Update dual-store migration comments for EDS-2 persist behavior5 files
Summary by CodeRabbit
Improvements
Tests