feat: support combined anonymous instances and OP#13 schema traits validation#13
Merged
Artifizer merged 3 commits intoGlobalTypeSystem:mainfrom Mar 6, 2026
Conversation
📝 WalkthroughWalkthroughUpdated submodule reference and package version; added UUID-tail handling in GTS ID parsing; implemented comprehensive trait- and inheritance-based schema validation in the store (with cycle detection and many helpers); added GTS.validateEntity and simplified server validate endpoint to call store.validateEntity. Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant Server
participant Store
participant Validator
Client->>Server: POST /validateEntity(id)
Server->>Store: validateEntity(id)
alt id refers to schema
Store->>Validator: validateSchemaAgainstParent(id)
Store->>Validator: validateSchemaTraits(id)
Validator-->>Store: ValidationResult (schema)
else id refers to instance
Store->>Validator: validateInstance(id) (resolve GTS id -> base id if needed)
Validator-->>Store: ValidationResult (instance)
end
Store-->>Server: ValidationResult + entity_type
Server-->>Client: 200 OK + ValidationResult
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Welcome to Codecov 🎉Once you merge this PR into your default branch, you're all set! Codecov will compare coverage reports and display results in all future pull requests. ℹ️ You can also turn on project coverage checks and project coverage reporting on Pull Request comment Thanks for integrating Codecov - We've got you covered ☂️ |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
src/store.ts (1)
98-104: Avoid double parsing by using try-catch pattern.
Gts.isValidGtsID(gtsId)internally callsparseGtsID(see context snippet 1), and then line 100 parses again. This duplicates work. Consider restructuring to parse once:♻️ Suggested refactor to avoid double parsing
- let objId: string = gtsId; - if (Gts.isValidGtsID(gtsId)) { - const gid = Gts.parseGtsID(gtsId); - objId = gid.id; - } + let objId: string; + try { + const gid = Gts.parseGtsID(gtsId); + objId = gid.id; + } catch { + // Not a valid GTS ID format, use raw input for lookup + objId = gtsId; + }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/store.ts` around lines 98 - 104, The code currently calls Gts.parseGtsID twice via Gts.isValidGtsID(gtsId) then again calling Gts.parseGtsID; change to call Gts.parseGtsID once in a try-catch: attempt const gid = Gts.parseGtsID(gtsId) and set objId = gid.id on success, and on failure fall back to objId = gtsId (or handle the error case as before); update the block around objId and the subsequent this.get(objId) to use the single parsed result and remove the Gts.isValidGtsID check to avoid duplicate parsing.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/store.ts`:
- Around line 98-104: The current lookup in src/store.ts lets arbitrary strings
bypass GTS validation by using objId = gtsId when Gts.isValidGtsID(gtsId) is
false; decide and implement one of two fixes: (A) if dual-format lookup is
intended, add an inline comment near the Gts.isValidGtsID / Gts.parseGtsID call
documenting that parseGtsID(gtsId).id returns the full GTS ID (not a short ID)
and that a raw string fallback is intentionally allowed for direct this.get
lookups, or (B) if validation is required, remove the conditional and enforce
validation by rejecting invalid IDs (e.g., throw or return an error) before
calling this.get; reference the symbols Gts.isValidGtsID, Gts.parseGtsID, objId,
and this.get when making the change.
---
Nitpick comments:
In `@src/store.ts`:
- Around line 98-104: The code currently calls Gts.parseGtsID twice via
Gts.isValidGtsID(gtsId) then again calling Gts.parseGtsID; change to call
Gts.parseGtsID once in a try-catch: attempt const gid = Gts.parseGtsID(gtsId)
and set objId = gid.id on success, and on failure fall back to objId = gtsId (or
handle the error case as before); update the block around objId and the
subsequent this.get(objId) to use the single parsed result and remove the
Gts.isValidGtsID check to avoid duplicate parsing.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 7afb0ca7-60d0-4706-8cc4-50a1e184717e
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (3)
.gts-specpackage.jsonsrc/store.ts
GeraBart
force-pushed
the
feat/anonymous_instances_test_OP#6
branch
from
5da2c0b to
0033f35
Compare
GeraBart
changed the title
GeraBart
force-pushed
the
feat/anonymous_instances_test_OP#6
branch
from
0033f35 to
7606154
Compare
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/gts.ts`:
- Around line 428-448: The UUID-tail branch currently treats the last token as
an anonymous UUID even when it's the only segment, causing "gts.<uuid>" to be
misclassified; update the conditional that checks "i === parts.length - 1 &&
!part.endsWith('~') && UUID_REGEX.test(part)" to also require an existing prior
segment (e.g., add "&& i > 0" or "&& gtsId.segments.length > 0") so a real
prefix is present before setting isUuidTail on the created GtsIDSegment; apply
the same change to the other identical UUID-tail block (the similar check around
the 466-470 area) to keep behavior consistent.
In `@src/store.ts`:
- Around line 1129-1133: The validateSchemaTraits(schemaId) call is placed after
the early-return for base schemas so base schemas skip OP#13; move or invoke
validateSchemaTraits (or call it conditionally before the early-return) so it
runs for schemas with no parent as well; update the validateEntity() /
validateSchema path to call validateSchemaTraits(schemaId) prior to the
base-schema early return (and keep validateEntityTraits() for completeness
checks) so malformed base trait schemas are validated across the inheritance
chain.
- Around line 1289-1296: The current early return in validateEntity() that
rejects entities when hasTraitValues is false wrongly rejects schemas whose
trait values are satisfied by defaults; instead of rejecting when hasTraitValues
is false, change the check to use the output of validateSchemaTraits() (or the
computed effective trait values it returns) — e.g., call or reuse
validateSchemaTraits() / its effectiveTraitValues result and only return the
error if required traits are still missing after defaults/materialization.
Update the logic around hasTraitValues and the error return (referencing
entityId, validateSchemaTraits(), and hasTraitValues) so defaults produced by
validateSchemaTraits() are considered valid rather than causing failure.
- Around line 1380-1389: The code in resolveTraitSchemaRefs silently discards
unresolved $ref when this.get(refId) returns undefined; update
resolveTraitSchemaRefs so that when refEntity is falsy (this.get(refId) returned
nothing) it does not drop the reference but instead surfaces the problem—e.g.,
throw a descriptive error or record an unresolved-ref failure that will cause
validation to fail (include refId and the parent id/context in the message).
Locate the block using this.get(refId), refEntity, resolveTraitSchemaRefs and
result, and replace the silent continue with logic that raises or propagates an
error for missing referenced entities so broken $ref entries do not become
looser schemas.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 45183d10-4214-419d-a77d-c6ef3bb9a700
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (6)
package.jsonsrc/gts.tssrc/index.tssrc/server/server.tssrc/store.tssrc/types.ts
GeraBart
force-pushed
the
feat/anonymous_instances_test_OP#6
branch
from
7606154 to
3527ae5
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (3)
src/store.ts (2)
1353-1360:⚠️ Potential issue | 🟠 MajorDon't reject trait schemas that are satisfied entirely by defaults.
This
!hasTraitValuesguard still fails schemas whose effective trait values are fully materialized byvalidateSchemaTraits(). Use the resolved trait-validation result here instead of requiring rawx-gts-traitsto be present.Proposed fix
- // If trait schemas exist but no trait values, entity is incomplete - if (!hasTraitValues) { - return { - id: entityId, - ok: false, - error: 'Entity defines x-gts-traits-schema but no x-gts-traits values are provided', - }; - } + const traitsResult = this.validateSchemaTraits(entityId); + if (!traitsResult.ok) { + return traitsResult; + }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/store.ts` around lines 1353 - 1360, The guard that rejects entities when !hasTraitValues should instead consider the resolved/validated trait result returned by validateSchemaTraits(); modify the logic around hasTraitValues to check the validation output (e.g., the result object returned by validateSchemaTraits() or its "ok"/"satisfied" flag) and only return the error for entityId when the validation result indicates the trait schema is not satisfied, allowing cases where defaults produced a fully materialized result to pass even if raw x-gts-traits is absent.
1449-1459:⚠️ Potential issue | 🟠 MajorUnresolvable trait refs are still silently discarded.
If
this.get(refId)misses here, the code just continues and drops the referenced constraints from the effective trait schema. That can turn a broken$refinto a passing validation.Proposed fix
const refEntity = this.get(refId); - if (refEntity && refEntity.content) { - const resolved = this.resolveTraitSchemaRefs(refEntity.content, visited, depth + 1); - // Merge resolved content into result - for (const [rk, rv] of Object.entries(resolved)) { - if (rk !== '$id' && rk !== '$$id' && rk !== '$schema' && rk !== '$$schema') { - result[rk] = rv; - } - } - } + if (!refEntity?.content) { + throw new Error(`Unresolvable GTS trait reference: ${refId}`); + } + const resolved = this.resolveTraitSchemaRefs(refEntity.content, visited, depth + 1); + for (const [rk, rv] of Object.entries(resolved)) { + if (rk !== '$id' && rk !== '$$id' && rk !== '$schema' && rk !== '$$schema') { + result[rk] = rv; + } + } continue;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/store.ts` around lines 1449 - 1459, When resolving $ref entries in resolveTraitSchemaRefs, don't silently skip missing references: if this.get(refId) returns falsy, throw a descriptive Error (or reject) that includes the missing refId and current context (e.g., caller trait id / visited stack) instead of continuing; update the block that currently reads refEntity = this.get(refId) to detect a missing ref and raise that error so callers of resolveTraitSchemaRefs (and any validation flow using it) fail fast rather than dropping constraints.src/gts.ts (1)
428-447:⚠️ Potential issue | 🟠 MajorRequire a real segment before treating the tail as a UUID.
Line 429 still lets
gts.<uuid>parse as a valid ID: the first/only segment enters the UUID-tail branch, and the single-segment instance guard at Lines 467-470 is then bypassed. Gate this path on an existing prior segment.Proposed fix
- if (i === parts.length - 1 && !part.endsWith('~') && UUID_REGEX.test(part)) { + if (i > 0 && i === parts.length - 1 && !part.endsWith('~') && UUID_REGEX.test(part)) {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/gts.ts` around lines 428 - 447, The UUID-tail branch currently allows a single-part ID like "gts.<uuid>" to be treated as a UUID tail; change the condition that enters the UUID tail handling to require an existing prior segment before accepting a UUID tail. Specifically, when checking "if (i === parts.length - 1 && !part.endsWith('~') && UUID_REGEX.test(part))" add a guard that gtsId.segments (or parts.length) indicates there was at least one real segment before the tail (e.g., require gtsId.segments.length > 0 or parts.length > 1) so the UUID-tail handling (GtsIDSegment created with isUuidTail = true) only runs when a prior segment exists.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/store.ts`:
- Around line 1517-1529: The detectRefCycle recursion shares the same visited
Set across sibling branches which can falsely mark separate branches as cyclic;
fix by cloning visited for each recursion branch: when recursing into a $ref
(refEntity.content) create a branchVisited = new Set(visited),
branchVisited.add(refId) and call this.detectRefCycle(originId,
refEntity.content, branchVisited, depth + 1); similarly, when iterating
content.allOf call this.detectRefCycle(originId, sub, new Set(visited), depth +
1) so each sibling branch gets its own visited set and cycles are only detected
along the actual path.
---
Duplicate comments:
In `@src/gts.ts`:
- Around line 428-447: The UUID-tail branch currently allows a single-part ID
like "gts.<uuid>" to be treated as a UUID tail; change the condition that enters
the UUID tail handling to require an existing prior segment before accepting a
UUID tail. Specifically, when checking "if (i === parts.length - 1 &&
!part.endsWith('~') && UUID_REGEX.test(part))" add a guard that gtsId.segments
(or parts.length) indicates there was at least one real segment before the tail
(e.g., require gtsId.segments.length > 0 or parts.length > 1) so the UUID-tail
handling (GtsIDSegment created with isUuidTail = true) only runs when a prior
segment exists.
In `@src/store.ts`:
- Around line 1353-1360: The guard that rejects entities when !hasTraitValues
should instead consider the resolved/validated trait result returned by
validateSchemaTraits(); modify the logic around hasTraitValues to check the
validation output (e.g., the result object returned by validateSchemaTraits() or
its "ok"/"satisfied" flag) and only return the error for entityId when the
validation result indicates the trait schema is not satisfied, allowing cases
where defaults produced a fully materialized result to pass even if raw
x-gts-traits is absent.
- Around line 1449-1459: When resolving $ref entries in resolveTraitSchemaRefs,
don't silently skip missing references: if this.get(refId) returns falsy, throw
a descriptive Error (or reject) that includes the missing refId and current
context (e.g., caller trait id / visited stack) instead of continuing; update
the block that currently reads refEntity = this.get(refId) to detect a missing
ref and raise that error so callers of resolveTraitSchemaRefs (and any
validation flow using it) fail fast rather than dropping constraints.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: d2e0355c-3df3-4680-89d5-c60ea3b51f9e
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (6)
package.jsonsrc/gts.tssrc/index.tssrc/server/server.tssrc/store.tssrc/types.ts
🚧 Files skipped from review as they are similar to previous changes (3)
- src/types.ts
- src/server/server.ts
- src/index.ts
…lidation Signed-off-by: GeraBart <246844849+GeraBart@users.noreply.github.com>
Signed-off-by: GeraBart <246844849+GeraBart@users.noreply.github.com>
GeraBart
force-pushed
the
feat/anonymous_instances_test_OP#6
branch
from
3527ae5 to
74710d4
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/gts.ts`:
- Line 18: Segments representing UUID tails must be skipped or handled
explicitly wherever version/identity fields are read: update code that inspects
segment fields to first check the isUuidTail flag and avoid treating its falsy
verMajor/verMinor or empty vendor/package/namespace/type as real values.
Concretely, in the logic that returns the "incomplete pattern" error (the
lastSeg check in the query handling where you test lastSeg.isType and
lastSeg.verMajor), add a guard to ensure you only apply that test when
!lastSeg.isUuidTail; in compatibility logic (the comparison code that directly
reads verMajor/verMinor) short-circuit or skip comparisons when seg.isUuidTail
is true; and in the cast/version-string builder (where you format
v{verMajor}.{verMinor}) detect isUuidTail and do not emit v0.0 but instead treat
it as a UUID-tail segment (skip or format as UUID). Apply these changes to all
uses of verMajor/verMinor/vendor/package/namespace/type to prevent
misclassification of UUID tail segments.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: a46c017c-1312-44c6-9365-af7366977bb2
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (6)
package.jsonsrc/gts.tssrc/index.tssrc/server/server.tssrc/store.tssrc/types.ts
🚧 Files skipped from review as they are similar to previous changes (3)
- src/types.ts
- package.json
- src/index.ts
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
3 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 by CodeRabbit
New Features
Bug Fixes
Chores