feat: implement metadata filtering in view predicates (Implements #2754)#2769
Conversation
🦋 Changeset detectedLatest commit: 554bf8e The changes in this PR will be included in the next version bump. This PR includes changesets to release 20 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 |
|
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:
📝 WalkthroughWalkthroughAdds metadata-aware filtering and propagation across grammar, parser, language server (grammar/semantic tokens/formatter), generators, core models, computed-view propagation and predicates, tests, examples, and syntax highlighting for Changes
Sequence Diagram(s)sequenceDiagram
participant User as DSL / Editor
participant Parser as Langium Parser
participant LSP as Language Server
participant Generator as LikeC4 Generator
participant Model as Core Model
participant ViewComp as View Computation
participant Predicate as Predicate Engine
participant Output as Computed Views
User->>Parser: Parse "where metadata.env is \"prod\""
Parser->>LSP: Emit AST (WhereMetadataEqual)
LSP->>Generator: Request formatting / schema handling
Generator->>ViewComp: Produce predicate representation
ViewComp->>Model: Read element/relation metadata via getters
Model-->>ViewComp: Return metadata record
ViewComp->>Predicate: Evaluate metadata predicate against record
Predicate-->>ViewComp: Match / No-match
ViewComp->>Output: Include or exclude node
Output->>User: Render filtered view
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
📝 Coding Plan
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 Tip You can validate your CodeRabbit configuration file in your editor.If your editor has YAML language server, you can enable auto-completion and validation by adding |
There was a problem hiding this comment.
Actionable comments posted: 5
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/core/src/compute-view/deployment-view/predicates/utils.ts (1)
250-258:⚠️ Potential issue | 🟠 MajorUse the deployed instance’s metadata in this enriched filterable shape.
This branch already enriches tags from
connectionEndpoint, butmetadatastill comes from the logical element.source/target metadatapredicates on model relations in deployment views will ignore instance overrides and can match the wrong connections.Suggested fix
return { kind: relationEndpoint.kind, tags: [...relationEndpoint.tags, ...(deployedInstance?.tags ?? [])], - metadata: relationEndpoint.metadata, + metadata: deployedInstance ? deployedInstance.metadata : relationEndpoint.metadata, }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/core/src/compute-view/deployment-view/predicates/utils.ts` around lines 250 - 258, The returned enriched shape for element relations currently uses relationEndpoint.metadata instead of instance overrides; change the metadata field to use the deployed instance's metadata when present by replacing relationEndpoint.metadata with deployedInstance?.metadata ?? relationEndpoint.metadata in the branch that checks isElementModel<M>(relationEndpoint) and computes deployedInstance (see isDeploymentElementModel<M>, isDeployedInstance, relationEndpoint, connectionEndpoint, deployedInstance, and the returned object with kind/tags/metadata).
🧹 Nitpick comments (4)
packages/language-server/src/model/parser/Base.ts (1)
248-255: Consider guarding against undefinedboolValue.If neither
p.valuenorp.boolValueis set (e.g., during parser error recovery or AST edge cases),String(p.boolValue)evaluates to"undefined", which is truthy and would pass the filter on line 255, resulting in metadata entries with the literal string"undefined".While the grammar suggests one must always be present, a defensive check would make this more robust.
🛡️ Proposed defensive fix
flatMap(p => { if (p.value) { return extractValues(p.value).map(v => [p.key, v] as [string, string]) } // Boolean metadata values (e.g. critical true) + if (p.boolValue !== undefined) { + return [[p.key, String(p.boolValue)] as [string, string]] + } - return [[p.key, String(p.boolValue)] as [string, string]] + return [] }),🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/language-server/src/model/parser/Base.ts` around lines 248 - 255, The flatMap/filter chain can emit ["key","undefined"] when neither p.value nor p.boolValue is present; modify the flatMap handling (around p.value / p.boolValue usage) to guard that p.boolValue is not undefined before returning String(p.boolValue) — only return the boolean branch if typeof p.boolValue !== "undefined" (or equivalent explicit check); otherwise return an empty array so the filter and downstream code never see the literal "undefined". Ensure this change is applied where extractValues, p.value and p.boolValue are referenced in the same mapping block.packages/core/src/types/view-computed.ts (1)
39-45: Extract metadata shape into a shared core type alias.Line 44 introduces an inline metadata record shape that is repeated in other changed TS files. Pull this into a shared exported type (e.g.,
ComputedMetadata) to avoid drift.♻️ Suggested refactor
+export type ComputedMetadata = Readonly<Record<string, string | string[] | undefined>> | null + export interface ComputedNode<A extends AnyAux = AnyAux> extends aux.WithTags<A>, aux.WithOptionalLinks { @@ - readonly metadata?: Readonly<Record<string, string | string[] | undefined>> | null + readonly metadata?: ComputedMetadataAs per coding guidelines,
**/*.{ts,tsx}: Agent types and interfaces should be defined separately and reused across the codebase.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/core/src/types/view-computed.ts` around lines 39 - 45, Extract the inline metadata record used on the metadata property into a shared exported type alias named ComputedMetadata; declare export type ComputedMetadata = Readonly<Record<string, string | string[] | undefined>> | null and replace the inline annotation on the metadata property (and any other duplicated occurrences across changed TS files) with ComputedMetadata, updating imports/exports so all modules reference the single alias instead of repeating the Record shape.packages/language-server/src/model/model-parser-where.ts (1)
5-16: Preferswitch(true)here to match repository control-flow style.The new helper works, but this block currently uses an if-chain.
🧩 Style-aligned alternative
const parseMetadataEquals = ( { operator, not }: ast.WhereMetadataEqual, value: string, ): c4.EqualOperator<string> => { - if (operator?.startsWith('!=')) { - return { neq: value } - } - if (operator?.startsWith('=')) { - return { eq: value } - } - return not ? { neq: value } : { eq: value } + switch (true) { + case operator?.startsWith('!='): + return { neq: value } + case operator?.startsWith('='): + return { eq: value } + default: + return not ? { neq: value } : { eq: value } + } }As per coding guidelines,
**/*.{ts,tsx,js,jsx}: Favor switch(true) over if-else chains.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/language-server/src/model/model-parser-where.ts` around lines 5 - 16, The parseMetadataEquals function uses an if-chain; refactor it to use a switch(true) style to match repository control-flow guidelines: inside parseMetadataEquals (accepting ast.WhereMetadataEqual and returning c4.EqualOperator<string>) replace the conditional checks for operator?.startsWith('!=') and operator?.startsWith('=') and the final not ? branch with a switch(true) that returns { neq: value } for the '!=’ case, { eq: value } for the '=' case, and the not ? { neq: value } : { eq: value } default; keep the same semantics and return types.packages/core/src/model/ElementModel.ts (1)
98-100: Use a cast-free metadata getter here as well.Line 99 has the same unsafe cast pattern; mirror the
return this.getMetadata()approach to keep this type-safe.As per coding guidelines,
**/*.{ts,tsx}: TypeScript-first repo; use explicit types. Avoid usingany, casts withas.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/core/src/model/ElementModel.ts` around lines 98 - 100, The metadata getter in ElementModel currently returns (this.$element.metadata ?? {}) as aux.Metadata<A>, which uses an unsafe cast; change the getter to return this.getMetadata() (i.e., delegate to the existing getMetadata() method) so the result is produced with proper typing instead of an explicit cast—update the metadata getter in class ElementModel to call getMetadata() and remove the casted return.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/core/src/model/DeploymentElementModel.ts`:
- Around line 443-449: The metadata getter now falls back to element.metadata
but AbstractDeploymentElementModel.hasMetadata() and getMetadata() still only
check this.$instance.metadata, causing inconsistent behavior; update
hasMetadata() and getMetadata(key) to consult the same fallback logic as the
metadata getter (i.e., check this.$instance.metadata first, then
this.element.metadata, then {}), using the same types as metadata() so that
hasMetadata() returns true when either instance or element metadata exist and
getMetadata(key) returns the value from instance metadata or, if missing, from
element.metadata.
In `@packages/core/src/model/RelationModel.ts`:
- Around line 126-128: The metadata getter in RelationModel currently returns
(this.$relationship.metadata ?? {}) as aux.Metadata<A>; change it to call and
return this.getMetadata() instead so you use the existing typed accessor
(getMetadata) rather than an unsafe cast; update the getter in the RelationModel
class to return this.getMetadata() and remove the casted expression.
In `@packages/core/src/types/operators.ts`:
- Around line 135-140: The metadata-existence check in the isMetadataEqual
branch uses the `key in item.metadata` operator which matches inherited
properties; update the predicate returned by isMetadataEqual (the block handling
operator.metadata when value === undefined) to use
Object.prototype.hasOwnProperty.call(item.metadata, key) and also ensure
item.metadata is non-null before calling it (preserve the existing
`item.metadata != null` check) so only own properties are considered when
testing existence.
In `@packages/language-server/src/lsp/SemanticTokenProvider.ts`:
- Around line 163-167: The current visitor block registers
ast.isWhereRelationKind in the isAnyOf check and then marks
mark.property('value').readonly.definition.type(), which duplicates the earlier
handling of WhereRelationKind and causes double semantic tokens; remove
ast.isWhereRelationKind from the isAnyOf list (or otherwise add a guard to skip
when the node was already handled) so only one visitor emits the token for
WhereRelationKind.value, keeping the
mark.property('value').readonly.definition.type() behavior for the remaining
kinds.
In `@packages/vscode/likec4.tmLanguage.json`:
- Around line 21-28: The regex in the "match" field currently uses
(is(\\s+not)?) which can match "is" as part of longer words (causing false
positives like "island"); update that capture to enforce word boundaries around
the "is" operator (for example replace the group with a version that uses
\\bis\\b and applies the optional "\\s+not" token with its own word boundary),
keeping the outer structure and the same capture index for the "is" token so the
captures 1/2/3 remain unchanged.
---
Outside diff comments:
In `@packages/core/src/compute-view/deployment-view/predicates/utils.ts`:
- Around line 250-258: The returned enriched shape for element relations
currently uses relationEndpoint.metadata instead of instance overrides; change
the metadata field to use the deployed instance's metadata when present by
replacing relationEndpoint.metadata with deployedInstance?.metadata ??
relationEndpoint.metadata in the branch that checks
isElementModel<M>(relationEndpoint) and computes deployedInstance (see
isDeploymentElementModel<M>, isDeployedInstance, relationEndpoint,
connectionEndpoint, deployedInstance, and the returned object with
kind/tags/metadata).
---
Nitpick comments:
In `@packages/core/src/model/ElementModel.ts`:
- Around line 98-100: The metadata getter in ElementModel currently returns
(this.$element.metadata ?? {}) as aux.Metadata<A>, which uses an unsafe cast;
change the getter to return this.getMetadata() (i.e., delegate to the existing
getMetadata() method) so the result is produced with proper typing instead of an
explicit cast—update the metadata getter in class ElementModel to call
getMetadata() and remove the casted return.
In `@packages/core/src/types/view-computed.ts`:
- Around line 39-45: Extract the inline metadata record used on the metadata
property into a shared exported type alias named ComputedMetadata; declare
export type ComputedMetadata = Readonly<Record<string, string | string[] |
undefined>> | null and replace the inline annotation on the metadata property
(and any other duplicated occurrences across changed TS files) with
ComputedMetadata, updating imports/exports so all modules reference the single
alias instead of repeating the Record shape.
In `@packages/language-server/src/model/model-parser-where.ts`:
- Around line 5-16: The parseMetadataEquals function uses an if-chain; refactor
it to use a switch(true) style to match repository control-flow guidelines:
inside parseMetadataEquals (accepting ast.WhereMetadataEqual and returning
c4.EqualOperator<string>) replace the conditional checks for
operator?.startsWith('!=') and operator?.startsWith('=') and the final not ?
branch with a switch(true) that returns { neq: value } for the '!=’ case, { eq:
value } for the '=' case, and the not ? { neq: value } : { eq: value } default;
keep the same semantics and return types.
In `@packages/language-server/src/model/parser/Base.ts`:
- Around line 248-255: The flatMap/filter chain can emit ["key","undefined"]
when neither p.value nor p.boolValue is present; modify the flatMap handling
(around p.value / p.boolValue usage) to guard that p.boolValue is not undefined
before returning String(p.boolValue) — only return the boolean branch if typeof
p.boolValue !== "undefined" (or equivalent explicit check); otherwise return an
empty array so the filter and downstream code never see the literal "undefined".
Ensure this change is applied where extractValues, p.value and p.boolValue are
referenced in the same mapping block.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: b71a8b51-c02a-480d-83bb-3db495d8c092
⛔ Files ignored due to path filters (50)
e2e/tests/__screenshots__/chromium-darwin/e2e-amazon-lambdas.pngis excluded by!**/*.pnge2e/tests/__screenshots__/chromium-darwin/e2e-amazon-rds.pngis excluded by!**/*.pnge2e/tests/__screenshots__/chromium-darwin/e2e-amazon-s3.pngis excluded by!**/*.pnge2e/tests/__screenshots__/chromium-darwin/e2e-amazon-sqs.pngis excluded by!**/*.pnge2e/tests/__screenshots__/chromium-darwin/e2e-amazon.pngis excluded by!**/*.pnge2e/tests/__screenshots__/chromium-darwin/e2e-backend.pngis excluded by!**/*.pnge2e/tests/__screenshots__/chromium-darwin/e2e-cloud-legacy-backend.pngis excluded by!**/*.pnge2e/tests/__screenshots__/chromium-darwin/e2e-cloud-legacy.pngis excluded by!**/*.pnge2e/tests/__screenshots__/chromium-darwin/e2e-cloud-next.pngis excluded by!**/*.pnge2e/tests/__screenshots__/chromium-darwin/e2e-cloud-to-amazon-sequence.pngis excluded by!**/*.pnge2e/tests/__screenshots__/chromium-darwin/e2e-cloud-to-amazon.pngis excluded by!**/*.pnge2e/tests/__screenshots__/chromium-darwin/e2e-cloud-ui-dashboard.pngis excluded by!**/*.pnge2e/tests/__screenshots__/chromium-darwin/e2e-cloud-ui-supportPanel.pngis excluded by!**/*.pnge2e/tests/__screenshots__/chromium-darwin/e2e-cloud-ui.pngis excluded by!**/*.pnge2e/tests/__screenshots__/chromium-darwin/e2e-cloud.pngis excluded by!**/*.pnge2e/tests/__screenshots__/chromium-darwin/e2e-customer.pngis excluded by!**/*.pnge2e/tests/__screenshots__/chromium-darwin/e2e-deploy-1.pngis excluded by!**/*.pnge2e/tests/__screenshots__/chromium-darwin/e2e-dynamic-view-1-sequence.pngis excluded by!**/*.pnge2e/tests/__screenshots__/chromium-darwin/e2e-dynamic-view-1.pngis excluded by!**/*.pnge2e/tests/__screenshots__/chromium-darwin/e2e-graphql.pngis excluded by!**/*.pnge2e/tests/__screenshots__/chromium-darwin/e2e-index.pngis excluded by!**/*.pnge2e/tests/__screenshots__/chromium-darwin/e2e-mobile.pngis excluded by!**/*.pnge2e/tests/__screenshots__/chromium-darwin/e2e-view-with-custom-colors.pngis excluded by!**/*.pnge2e/tests/__screenshots__/chromium-darwin/issue-2282-index.pngis excluded by!**/*.pnge2e/tests/__screenshots__/chromium-linux/e2e-amazon-lambdas.pngis excluded by!**/*.pnge2e/tests/__screenshots__/chromium-linux/e2e-amazon-rds.pngis excluded by!**/*.pnge2e/tests/__screenshots__/chromium-linux/e2e-amazon-s3.pngis excluded by!**/*.pnge2e/tests/__screenshots__/chromium-linux/e2e-amazon-sqs.pngis excluded by!**/*.pnge2e/tests/__screenshots__/chromium-linux/e2e-amazon.pngis excluded by!**/*.pnge2e/tests/__screenshots__/chromium-linux/e2e-backend.pngis excluded by!**/*.pnge2e/tests/__screenshots__/chromium-linux/e2e-cloud-legacy-backend.pngis excluded by!**/*.pnge2e/tests/__screenshots__/chromium-linux/e2e-cloud-legacy.pngis excluded by!**/*.pnge2e/tests/__screenshots__/chromium-linux/e2e-cloud-next.pngis excluded by!**/*.pnge2e/tests/__screenshots__/chromium-linux/e2e-cloud-to-amazon-sequence.pngis excluded by!**/*.pnge2e/tests/__screenshots__/chromium-linux/e2e-cloud-to-amazon.pngis excluded by!**/*.pnge2e/tests/__screenshots__/chromium-linux/e2e-cloud-ui-dashboard.pngis excluded by!**/*.pnge2e/tests/__screenshots__/chromium-linux/e2e-cloud-ui-supportPanel.pngis excluded by!**/*.pnge2e/tests/__screenshots__/chromium-linux/e2e-cloud-ui.pngis excluded by!**/*.pnge2e/tests/__screenshots__/chromium-linux/e2e-cloud.pngis excluded by!**/*.pnge2e/tests/__screenshots__/chromium-linux/e2e-customer.pngis excluded by!**/*.pnge2e/tests/__screenshots__/chromium-linux/e2e-deploy-1.pngis excluded by!**/*.pnge2e/tests/__screenshots__/chromium-linux/e2e-dynamic-view-1-sequence.pngis excluded by!**/*.pnge2e/tests/__screenshots__/chromium-linux/e2e-dynamic-view-1.pngis excluded by!**/*.pnge2e/tests/__screenshots__/chromium-linux/e2e-graphql.pngis excluded by!**/*.pnge2e/tests/__screenshots__/chromium-linux/e2e-index.pngis excluded by!**/*.pnge2e/tests/__screenshots__/chromium-linux/e2e-mobile.pngis excluded by!**/*.pnge2e/tests/__screenshots__/chromium-linux/e2e-view-with-custom-colors.pngis excluded by!**/*.pnge2e/tests/__screenshots__/chromium-linux/issue-2282-index.pngis excluded by!**/*.pngpackages/generators/src/model/__snapshots__/likec4.computed-model-with-core.snapis excluded by!**/*.snappackages/generators/src/model/__snapshots__/likec4.computed-model.snapis excluded by!**/*.snap
📒 Files selected for processing (26)
apps/docs/likec4.tmLanguage.jsonapps/docs/src/content/docs/dsl/Views/predicates.mdxapps/playground/likec4.tmLanguage.jsonexamples/metadata-views/likec4.config.jsonexamples/metadata-views/views.c4packages/core/src/compute-view/deployment-view/predicates/utils.tspackages/core/src/compute-view/utils/buildComputedNodes.tspackages/core/src/compute-view/utils/relationExpressionToPredicates.tspackages/core/src/model/DeploymentElementModel.tspackages/core/src/model/ElementModel.tspackages/core/src/model/RelationModel.tspackages/core/src/types/operators.spec.tspackages/core/src/types/operators.tspackages/core/src/types/view-computed.tspackages/generators/src/likec4/operators/expressions.spec.tspackages/generators/src/likec4/operators/expressions.tspackages/generators/src/likec4/schemas/expression.tspackages/language-server/src/__tests__/deployment-views.spec.tspackages/language-server/src/__tests__/views-2.spec.tspackages/language-server/src/formatting/LikeC4Formatter.tspackages/language-server/src/like-c4.langiumpackages/language-server/src/lsp/SemanticTokenProvider.tspackages/language-server/src/model/model-parser-where.tspackages/language-server/src/model/parser/Base.tspackages/likec4/src/LikeC4.spec.tspackages/vscode/likec4.tmLanguage.json
| /** | ||
| * Instance metadata overrides element metadata entirely (replacement, not merge). | ||
| * This differs from tags which are merged from instance + element. | ||
| */ | ||
| get metadata(): aux.Metadata<A> { | ||
| return (this.$instance.metadata ?? this.element.metadata ?? {}) as aux.Metadata<A> | ||
| } |
There was a problem hiding this comment.
Keep the metadata helpers consistent with the new instance fallback.
metadata now falls back to element.metadata, but AbstractDeploymentElementModel.hasMetadata() / getMetadata() still read only this.$instance.metadata. An instance without local metadata will expose metadata while reporting hasMetadata() === false and getMetadata('...') === undefined.
Suggested fix
abstract class AbstractDeploymentElementModel<A extends Any> implements WithTags<A>, WithMetadata<A> {
+ abstract get metadata(): aux.Metadata<A>
+
public hasMetadata(): boolean {
- return !!this.$node.metadata && !isEmpty(this.$node.metadata)
+ return !isEmpty(this.metadata)
}
public getMetadata(): aux.Metadata<A>
public getMetadata(field: aux.MetadataKey<A>): string | string[] | undefined
public getMetadata(field?: aux.MetadataKey<A>) {
if (field) {
- return this.$node.metadata?.[field]
+ return this.metadata[field]
}
- return this.$node.metadata ?? {}
+ return this.metadata
}
}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/core/src/model/DeploymentElementModel.ts` around lines 443 - 449,
The metadata getter now falls back to element.metadata but
AbstractDeploymentElementModel.hasMetadata() and getMetadata() still only check
this.$instance.metadata, causing inconsistent behavior; update hasMetadata() and
getMetadata(key) to consult the same fallback logic as the metadata getter
(i.e., check this.$instance.metadata first, then this.element.metadata, then
{}), using the same types as metadata() so that hasMetadata() returns true when
either instance or element metadata exist and getMetadata(key) returns the value
from instance metadata or, if missing, from element.metadata.
| get metadata(): aux.Metadata<A> { | ||
| return (this.$relationship.metadata ?? {}) as aux.Metadata<A> | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
rg -n "as aux\\.Metadata<" packages/core/src/model/RelationModel.ts packages/core/src/model/ElementModel.tsRepository: likec4/likec4
Length of output: 271
🏁 Script executed:
#!/bin/bash
# Get the context around the metadata getter and examine the class structure
sed -n '100,150p' packages/core/src/model/RelationModel.ts | cat -nRepository: likec4/likec4
Length of output: 1814
🏁 Script executed:
#!/bin/bash
# Check if getMetadata() method exists in RelationModel.ts
rg -n "getMetadata" packages/core/src/model/RelationModel.ts -A 2Repository: likec4/likec4
Length of output: 313
🏁 Script executed:
#!/bin/bash
# Compare with ElementModel.ts to see the pattern
rg -n "metadata" packages/core/src/model/ElementModel.ts | head -20Repository: likec4/likec4
Length of output: 328
🏁 Script executed:
#!/bin/bash
# Get the full getMetadata() implementation in RelationModel.ts
sed -n '174,185p' packages/core/src/model/RelationModel.ts | cat -nRepository: likec4/likec4
Length of output: 496
Replace the as aux.Metadata<A> cast with the existing getMetadata() method.
At line 127, use this.getMetadata() instead of casting. The method already returns the correct type without requiring an unsafe cast.
get metadata(): aux.Metadata<A> {
- return (this.$relationship.metadata ?? {}) as aux.Metadata<A>
+ return this.getMetadata()
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| get metadata(): aux.Metadata<A> { | |
| return (this.$relationship.metadata ?? {}) as aux.Metadata<A> | |
| } | |
| get metadata(): aux.Metadata<A> { | |
| return this.getMetadata() | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/core/src/model/RelationModel.ts` around lines 126 - 128, The
metadata getter in RelationModel currently returns (this.$relationship.metadata
?? {}) as aux.Metadata<A>; change it to call and return this.getMetadata()
instead so you use the existing typed accessor (getMetadata) rather than an
unsafe cast; update the getter in the RelationModel class to return
this.getMetadata() and remove the casted expression.
| case isMetadataEqual(operator): { | ||
| const { key, value } = operator.metadata | ||
| if (value === undefined) { | ||
| return (item) => { | ||
| return item.metadata != null && key in item.metadata && item.metadata[key] != null | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, check the exact code at lines 135-140
cat -n packages/core/src/types/operators.ts | sed -n '130,150p'Repository: likec4/likec4
Length of output: 942
🏁 Script executed:
# Check the full metadata operator implementation context
cat -n packages/core/src/types/operators.ts | sed -n '120,165p'Repository: likec4/likec4
Length of output: 1998
🏁 Script executed:
# Check the type definitions for metadata
rg -A 5 -B 5 "metadata.*:" packages/core/src/types/operators.ts | head -60Repository: likec4/likec4
Length of output: 1006
🏁 Script executed:
# Check what the metadata structure is expected to be
rg "item\.metadata" packages/core/src/types/operators.ts -A 2 -B 2Repository: likec4/likec4
Length of output: 764
🏁 Script executed:
# Check if there are other places in the codebase using similar patterns
rg "in .*metadata" --type ts --type tsxRepository: likec4/likec4
Length of output: 84
🏁 Script executed:
# Let me search for other patterns with metadata checks using correct flags
rg "in.*metadata" packages/core/src/types/ -A 2 -B 2Repository: likec4/likec4
Length of output: 9352
🏁 Script executed:
# Check how metadata objects are created/populated in the codebase
rg "metadata\s*=" packages/core/src/ -B 2 -A 2 | head -80Repository: likec4/likec4
Length of output: 39
🏁 Script executed:
# Look for any test cases that might show expected behavior
find packages/core -name "*.test.*" -o -name "*.spec.*" | xargs grep -l "metadata" 2>/dev/null | head -5Repository: likec4/likec4
Length of output: 156
🏁 Script executed:
# Check if metadata is ever accessed with prototype-chain properties
rg "metadata\[.*constructor" packages/core/src/Repository: likec4/likec4
Length of output: 39
🏁 Script executed:
# Verify the exact type definition of metadata to understand if prototype pollution is expected
cat -n packages/core/src/types/operators.ts | sed -n '85,100p'Repository: likec4/likec4
Length of output: 612
🏁 Script executed:
# Check if there are other property-in checks in the codebase that might have same issue
rg " in " packages/core/src/types/operators.tsRepository: likec4/likec4
Length of output: 519
🏁 Script executed:
# Look at the test for metadata existence check to understand expected behavior
cat -n packages/core/src/types/operators.spec.ts | grep -A 15 "metadata existence check"Repository: likec4/likec4
Length of output: 1519
🏁 Script executed:
# Check if metadata objects are always plain objects or could have custom prototypes
rg "Object.create\|Object.setPrototypeOf" packages/core/src/Repository: likec4/likec4
Length of output: 39
🏁 Script executed:
# Search for any documented behavior or comments about metadata handling
rg -B 5 "metadata.*key" packages/core/src/types/ | head -50Repository: likec4/likec4
Length of output: 3825
Use hasOwnProperty() for own-key-only metadata checks.
Line 139 uses key in item.metadata, which walks the prototype chain. Filters checking for inherited property names like constructor or toString would incorrectly match. Use Object.prototype.hasOwnProperty.call() instead.
Suggested fix
if (value === undefined) {
return (item) => {
- return item.metadata != null && key in item.metadata && item.metadata[key] != null
+ return item.metadata != null
+ && Object.prototype.hasOwnProperty.call(item.metadata, key)
+ && item.metadata[key] != null
}
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| case isMetadataEqual(operator): { | |
| const { key, value } = operator.metadata | |
| if (value === undefined) { | |
| return (item) => { | |
| return item.metadata != null && key in item.metadata && item.metadata[key] != null | |
| } | |
| case isMetadataEqual(operator): { | |
| const { key, value } = operator.metadata | |
| if (value === undefined) { | |
| return (item) => { | |
| return item.metadata != null | |
| && Object.prototype.hasOwnProperty.call(item.metadata, key) | |
| && item.metadata[key] != null | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/core/src/types/operators.ts` around lines 135 - 140, The
metadata-existence check in the isMetadataEqual branch uses the `key in
item.metadata` operator which matches inherited properties; update the predicate
returned by isMetadataEqual (the block handling operator.metadata when value ===
undefined) to use Object.prototype.hasOwnProperty.call(item.metadata, key) and
also ensure item.metadata is non-null before calling it (preserve the existing
`item.metadata != null` check) so only own properties are considered when
testing existence.
| { | ||
| "match": "\\b(metadata)\\.([a-zA-Z_]\\w*)(?:\\s+(is(\\s+not)?))?", | ||
| "captures": { | ||
| "1": { "name": "keyword.control.likec4" }, | ||
| "2": { "name": "variable.other.property.likec4" }, | ||
| "3": { "name": "keyword.control.likec4" } | ||
| } | ||
| }, |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
python - <<'PY'
import re
p = re.compile(r'\b(metadata)\.([a-zA-Z_]\w*)(?:\s+(is(\s+not)?))?')
tests = [
'metadata.env',
'metadata.env is prod',
'metadata.env is not prod',
'metadata.env island'
]
for t in tests:
m = p.search(t)
print(f'{t!r} -> {m.group(0)!r}' if m else f'{t!r} -> None')
PYRepository: likec4/likec4
Length of output: 228
🏁 Script executed:
# Check if the file exists and view lines around 21-28
fd likec4.tmLanguage.json -x wc -l {}Repository: likec4/likec4
Length of output: 185
🏁 Script executed:
# Read lines 21-28 from the file
fd likec4.tmLanguage.json -x sed -n '20,30p' {}Repository: likec4/likec4
Length of output: 965
🏁 Script executed:
# Test the suggested regex fix
python - <<'PY'
import re
# Original regex (current)
original = re.compile(r'\b(metadata)\.([a-zA-Z_]\w*)(?:\s+(is(\s+not)?))?')
# Suggested fix (with trailing \b and non-capturing group)
suggested = re.compile(r'\b(metadata)\.([a-zA-Z_]\w*)(?:\s+(is(?:\s+not)?))?\\b')
tests = [
'metadata.env',
'metadata.env is prod',
'metadata.env is not prod',
'metadata.env island',
'metadata.description is important',
]
print("ORIGINAL REGEX:")
for t in tests:
m = original.search(t)
print(f' {t!r:40} -> {m.group(0)!r if m else "None"}')
print("\nSUGGESTED REGEX (with trailing \\b):")
for t in tests:
m = suggested.search(t)
print(f' {t!r:40} -> {m.group(0)!r if m else "None"}')
PYRepository: likec4/likec4
Length of output: 239
🏁 Script executed:
python - <<'PY'
import re
# Original regex (current)
original = re.compile(r'\b(metadata)\.([a-zA-Z_]\w*)(?:\s+(is(\s+not)?))?')
# Suggested fix (with trailing \b and non-capturing group)
suggested = re.compile(r'\b(metadata)\.([a-zA-Z_]\w*)(?:\s+(is(?:\s+not)?))?\\b')
tests = [
'metadata.env',
'metadata.env is prod',
'metadata.env is not prod',
'metadata.env island',
'metadata.description is important',
]
print("ORIGINAL REGEX:")
for t in tests:
m = original.search(t)
result = m.group(0) if m else "None"
print(" " + repr(t).ljust(40) + " -> " + repr(result))
print("\nSUGGESTED REGEX (with trailing \\b):")
for t in tests:
m = suggested.search(t)
result = m.group(0) if m else "None"
print(" " + repr(t).ljust(40) + " -> " + repr(result))
PYRepository: likec4/likec4
Length of output: 703
🏁 Script executed:
# Verify JSON formatting with dprint
fd likec4.tmLanguage.json -path "*/packages/vscode/*" -x cat {}Repository: likec4/likec4
Length of output: 225
🏁 Script executed:
# Read the actual file content around the metadata regex
sed -n '15,35p' ./packages/vscode/likec4.tmLanguage.jsonRepository: likec4/likec4
Length of output: 672
🏁 Script executed:
# Test the correct fix: word boundary BEFORE 'is'
python - <<'PY'
import re
# Original regex (current) - no word boundary around 'is'
original = re.compile(r'\b(metadata)\.([a-zA-Z_]\w*)(?:\s+(is(\s+not)?))?')
# Correct fix: word boundary BEFORE 'is'
correct_fix = re.compile(r'\b(metadata)\.([a-zA-Z_]\w*)(?:\s+\b(is)(?:\s+not)?\b)?')
tests = [
'metadata.env',
'metadata.env is prod',
'metadata.env is not prod',
'metadata.env island',
'metadata.description is important',
]
print("ORIGINAL (false positive issue):")
for t in tests:
m = original.search(t)
result = m.group(0) if m else "None"
print(" " + repr(t).ljust(40) + " -> " + repr(result))
print("\nCORRECT FIX (word boundary before 'is'):")
for t in tests:
m = correct_fix.search(t)
result = m.group(0) if m else "None"
print(" " + repr(t).ljust(40) + " -> " + repr(result))
PYRepository: likec4/likec4
Length of output: 786
🏁 Script executed:
# Check for similar "is" patterns without proper boundaries
grep -n "is" ./packages/vscode/likec4.tmLanguage.json | head -20Repository: likec4/likec4
Length of output: 737
🏁 Script executed:
# Verify JSON formatting is valid
python - <<'PY'
import json
with open('./packages/vscode/likec4.tmLanguage.json') as f:
content = json.load(f)
print("✓ JSON is valid")
PYRepository: likec4/likec4
Length of output: 72
Add word boundaries around the is keyword to prevent false-positive matches.
At Line 22, the is operator can be incorrectly matched as a prefix inside longer words (e.g., metadata.env island matches metadata.env is), causing incorrect syntax highlighting.
🔧 Suggested regex fix
- "match": "\\b(metadata)\\.([a-zA-Z_]\\w*)(?:\\s+(is(\\s+not)?))?",
+ "match": "\\b(metadata)\\.([a-zA-Z_]\\w*)(?:\\s+\\b(is)(?:\\s+not)?\\b)?",📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| { | |
| "match": "\\b(metadata)\\.([a-zA-Z_]\\w*)(?:\\s+(is(\\s+not)?))?", | |
| "captures": { | |
| "1": { "name": "keyword.control.likec4" }, | |
| "2": { "name": "variable.other.property.likec4" }, | |
| "3": { "name": "keyword.control.likec4" } | |
| } | |
| }, | |
| { | |
| "match": "\\b(metadata)\\.([a-zA-Z_]\\w*)(?:\\s+\\b(is)(?:\\s+not)?\\b)?", | |
| "captures": { | |
| "1": { "name": "keyword.control.likec4" }, | |
| "2": { "name": "variable.other.property.likec4" }, | |
| "3": { "name": "keyword.control.likec4" } | |
| } | |
| }, |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/vscode/likec4.tmLanguage.json` around lines 21 - 28, The regex in
the "match" field currently uses (is(\\s+not)?) which can match "is" as part of
longer words (causing false positives like "island"); update that capture to
enforce word boundaries around the "is" operator (for example replace the group
with a version that uses \\bis\\b and applies the optional "\\s+not" token with
its own word boundary), keeping the outer structure and the same capture index
for the "is" token so the captures 1/2/3 remain unchanged.
davydkov
left a comment
There was a problem hiding this comment.
Hey @galuszkak
You did nice job, and everything looks pretty solid, but I'd like to run it locally first and check if code completions are not impacted (from the code it seems it should not)
In Claude you can also trigger changeset-generator skill - this helps with release notes
Can you also revert regenerated E2E screenshots? There is a github workflow to regenerate screenshots using same consistent env, but I don't expect any failures.
And yes, seems I need to update PR checks list.
1224211 to
01bf60b
Compare
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/language-server/src/like-c4.langium (1)
609-650:⚠️ Potential issue | 🟡 MinorUpdate TextMate grammars to support
participant.metadatasyntax.The
pnpm generatestep appears complete (generated AST types are in use). However, TextMate grammars need updates for the newparticipant.metadata.keywhere clause syntax. Currently, all three grammar files only include the pattern formetadata.key, but the new grammar adds support for filtering by relationship participant metadata (e.g.,source.metadata.env = production).Add patterns to
packages/vscode/likec4.tmLanguage.json,apps/docs/likec4.tmLanguage.json, andapps/playground/likec4.tmLanguage.jsonto recognizeparticipant.metadatasyntax in where clauses.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/language-server/src/like-c4.langium` around lines 609 - 650, The TextMate grammars are missing a pattern for the new participant.metadata where-clause syntax introduced by WhereRelationParticipantMetadata (e.g., "participant.metadata.key = value"); update the three grammar files (packages/vscode/likec4.tmLanguage.json, apps/docs/likec4.tmLanguage.json, apps/playground/likec4.tmLanguage.json) to add a pattern that matches "participant" followed by ".metadata" and a dotted key (Id) in the where/relation context (analogous to the existing metadata.key pattern), assign the same scopes used for metadata keys and operators so occurrences of participant.metadata.<key> are highlighted like metadata.key, and ensure the pattern is included in the where/relationship-related repository entries so WhereRelationParticipantMetadata instances are recognized.
♻️ Duplicate comments (2)
packages/vscode/likec4.tmLanguage.json (1)
21-28:⚠️ Potential issue | 🟡 MinorAlign the metadata matcher with the DSL grammar.
This regex still accepts invalid keys like
metadata._1, misses valid ones likemetadata.some-key, and can consumeisfrommetadata.env islandas if it were the operator. Tighten the identifier part to the DSL’s[_]*[a-zA-Z][-\w]*shape and put word boundaries aroundis. Please mirror the same fix inapps/docs/likec4.tmLanguage.jsonandapps/playground/likec4.tmLanguage.json.🔧 Suggested fix
- "match": "\\b(metadata)\\.([a-zA-Z_]\\w*)(?:\\s+(is(\\s+not)?))?", + "match": "\\b(metadata)\\.([_]*[a-zA-Z][-\\w]*)(?:\\s+\\b(is(?:\\s+not)?)\\b)?",Run this to reproduce the current mismatches and compare the tightened pattern:
#!/bin/bash python - <<'PY' import re current = re.compile(r'\b(metadata)\.([a-zA-Z_]\w*)(?:\s+(is(\s+not)?))?') fixed = re.compile(r'\b(metadata)\.([_]*[a-zA-Z][-\w]*)(?:\s+\b(is(?:\s+not)?)\b)?') tests = [ 'metadata.env', 'metadata.__env', 'metadata.some-key', 'metadata._1', 'metadata.env is true', 'metadata.env is not false', 'metadata.env island', ] for label, pattern in [('current', current), ('fixed', fixed)]: print(label.upper()) for text in tests: match = pattern.search(text) print(f' {text!r:<30} -> {match.group(0)!r if match else None}') print() PY🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/vscode/likec4.tmLanguage.json` around lines 21 - 28, Update the "match" regex for the metadata rule so the identifier portion follows the DSL shape and the "is" operator is bounded: replace the existing pattern \b(metadata)\.([a-zA-Z_]\w*)(?:\s+(is(\s+not)?))? with a tightened pattern that uses [_]*[a-zA-Z][-\w]* for capture group 2 and wraps the operator in word boundaries (e.g. (?:\s+\b(is(?:\s+not)?)\b)?); apply this exact change to the same "match" entry in the other two grammar files (apps/docs/likec4.tmLanguage.json and apps/playground/likec4.tmLanguage.json) so all three metadata matchers are consistent.packages/language-server/src/lsp/SemanticTokenProvider.ts (1)
162-169:⚠️ Potential issue | 🟠 MajorDrop
ast.isWhereRelationKindfrom this grouped rule.Line 150 already highlights
WhereRelationKind.valueasfunction(). Keeping it here emits a second token over the same range astype(), so relation-kind predicates get conflicting semantic tokens.🔧 Suggested fix
when( - isAnyOf(ast.isWhereElementTag, ast.isWhereElementKind, ast.isWhereRelationTag, ast.isWhereRelationKind), + isAnyOf(ast.isWhereElementTag, ast.isWhereElementKind, ast.isWhereRelationTag), mark => { if (isTruthy(mark.node.value)) { mark.property('value').readonly.definition.type() } }, )🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/language-server/src/lsp/SemanticTokenProvider.ts` around lines 162 - 169, Remove ast.isWhereRelationKind from the grouped predicate passed to isAnyOf in the when(...) call so it no longer applies the same tokenization twice; update the isAnyOf call to only include ast.isWhereElementTag, ast.isWhereElementKind, and ast.isWhereRelationTag so that the mark.property('value').readonly.definition.type() branch does not run for WhereRelationKind nodes (which are already tokenized elsewhere).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In `@packages/language-server/src/like-c4.langium`:
- Around line 609-650: The TextMate grammars are missing a pattern for the new
participant.metadata where-clause syntax introduced by
WhereRelationParticipantMetadata (e.g., "participant.metadata.key = value");
update the three grammar files (packages/vscode/likec4.tmLanguage.json,
apps/docs/likec4.tmLanguage.json, apps/playground/likec4.tmLanguage.json) to add
a pattern that matches "participant" followed by ".metadata" and a dotted key
(Id) in the where/relation context (analogous to the existing metadata.key
pattern), assign the same scopes used for metadata keys and operators so
occurrences of participant.metadata.<key> are highlighted like metadata.key, and
ensure the pattern is included in the where/relationship-related repository
entries so WhereRelationParticipantMetadata instances are recognized.
---
Duplicate comments:
In `@packages/language-server/src/lsp/SemanticTokenProvider.ts`:
- Around line 162-169: Remove ast.isWhereRelationKind from the grouped predicate
passed to isAnyOf in the when(...) call so it no longer applies the same
tokenization twice; update the isAnyOf call to only include
ast.isWhereElementTag, ast.isWhereElementKind, and ast.isWhereRelationTag so
that the mark.property('value').readonly.definition.type() branch does not run
for WhereRelationKind nodes (which are already tokenized elsewhere).
In `@packages/vscode/likec4.tmLanguage.json`:
- Around line 21-28: Update the "match" regex for the metadata rule so the
identifier portion follows the DSL shape and the "is" operator is bounded:
replace the existing pattern \b(metadata)\.([a-zA-Z_]\w*)(?:\s+(is(\s+not)?))?
with a tightened pattern that uses [_]*[a-zA-Z][-\w]* for capture group 2 and
wraps the operator in word boundaries (e.g. (?:\s+\b(is(?:\s+not)?)\b)?); apply
this exact change to the same "match" entry in the other two grammar files
(apps/docs/likec4.tmLanguage.json and apps/playground/likec4.tmLanguage.json) so
all three metadata matchers are consistent.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: b8b3edbc-a254-406e-af77-79710c9358d4
⛔ Files ignored due to path filters (3)
e2e/tests/__screenshots__/chromium-darwin/e2e-amazon-s3.pngis excluded by!**/*.pngpackages/generators/src/model/__snapshots__/likec4.computed-model-with-core.snapis excluded by!**/*.snappackages/generators/src/model/__snapshots__/likec4.computed-model.snapis excluded by!**/*.snap
📒 Files selected for processing (26)
apps/docs/likec4.tmLanguage.jsonapps/docs/src/content/docs/dsl/Views/predicates.mdxapps/playground/likec4.tmLanguage.jsonexamples/metadata-views/likec4.config.jsonexamples/metadata-views/views.c4packages/core/src/compute-view/deployment-view/predicates/utils.tspackages/core/src/compute-view/utils/buildComputedNodes.tspackages/core/src/compute-view/utils/relationExpressionToPredicates.tspackages/core/src/model/DeploymentElementModel.tspackages/core/src/model/ElementModel.tspackages/core/src/model/RelationModel.tspackages/core/src/types/operators.spec.tspackages/core/src/types/operators.tspackages/core/src/types/view-computed.tspackages/generators/src/likec4/operators/expressions.spec.tspackages/generators/src/likec4/operators/expressions.tspackages/generators/src/likec4/schemas/expression.tspackages/language-server/src/__tests__/deployment-views.spec.tspackages/language-server/src/__tests__/views-2.spec.tspackages/language-server/src/formatting/LikeC4Formatter.tspackages/language-server/src/like-c4.langiumpackages/language-server/src/lsp/SemanticTokenProvider.tspackages/language-server/src/model/model-parser-where.tspackages/language-server/src/model/parser/Base.tspackages/likec4/src/LikeC4.spec.tspackages/vscode/likec4.tmLanguage.json
🚧 Files skipped from review as they are similar to previous changes (13)
- packages/likec4/src/LikeC4.spec.ts
- packages/language-server/src/tests/deployment-views.spec.ts
- packages/core/src/compute-view/utils/buildComputedNodes.ts
- packages/generators/src/likec4/operators/expressions.ts
- packages/core/src/model/RelationModel.ts
- packages/language-server/src/model/model-parser-where.ts
- packages/generators/src/likec4/schemas/expression.ts
- packages/core/src/types/view-computed.ts
- packages/core/src/types/operators.spec.ts
- examples/metadata-views/views.c4
- apps/docs/src/content/docs/dsl/Views/predicates.mdx
- packages/generators/src/likec4/operators/expressions.spec.ts
- packages/language-server/src/model/parser/Base.ts
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 @.changeset/metadata-views-filtering.md:
- Around line 2-3: The changeset .changeset/metadata-views-filtering.md
incorrectly marks the package bumps for '@likec4/core' and
'@likec4/language-server' as patch despite adding new feature functionality
(metadata filtering in view predicates); update the changeset to use minor
version bumps for both packages by changing their entries from 'patch' to
'minor' so the releases follow semantic versioning for backward-compatible
features.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 1b94e3c3-7097-43fa-a4b1-54120a7b0302
📒 Files selected for processing (1)
.changeset/metadata-views-filtering.md
dc74c99 to
554bf8e
Compare
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/generators/src/likec4/schemas/expression.ts`:
- Around line 35-38: The whereMetadata schema currently restricts metadata.value
to strings via equalOp(z.string()).optional(), causing boolean predicates like
metadata.critical is true to be rejected; update the schema so
whereMetadata.value accepts both strings and booleans (e.g. replace
equalOp(z.string()) with equalOp(z.union([z.string(), z.boolean()])) or
equivalent) so the equalOp validation allows boolean metadata comparisons while
keeping the field optional.
In `@packages/likec4/src/LikeC4.spec.ts`:
- Around line 118-144: The current tests for views (using
model.view('prodOnly').elements(), 'excludeStaging', 'criticalOnly',
'prodNotDb') only assert subsets via toContain / not.toContain; change each
block to assert the exact set of element IDs by mapping elements to ids,
sorting, and comparing against the expected sorted array (e.g., assert that
prodOnlyIds sorted equals ['container.prodApi','container.prodDb'] and similarly
tighten excludeStaging, criticalOnly, and prodNotDb to exact expected ID arrays)
so over-inclusion will fail the test.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: dce76dd8-4c8f-4c8d-b211-0650b84d95b9
⛔ Files ignored due to path filters (3)
e2e/tests/__screenshots__/chromium-darwin/e2e-amazon-s3.pngis excluded by!**/*.pngpackages/generators/src/model/__snapshots__/likec4.computed-model-with-core.snapis excluded by!**/*.snappackages/generators/src/model/__snapshots__/likec4.computed-model.snapis excluded by!**/*.snap
📒 Files selected for processing (27)
.changeset/metadata-views-filtering.mdapps/docs/likec4.tmLanguage.jsonapps/docs/src/content/docs/dsl/Views/predicates.mdxapps/playground/likec4.tmLanguage.jsonexamples/metadata-views/likec4.config.jsonexamples/metadata-views/views.c4packages/core/src/compute-view/deployment-view/predicates/utils.tspackages/core/src/compute-view/utils/buildComputedNodes.tspackages/core/src/compute-view/utils/relationExpressionToPredicates.tspackages/core/src/model/DeploymentElementModel.tspackages/core/src/model/ElementModel.tspackages/core/src/model/RelationModel.tspackages/core/src/types/operators.spec.tspackages/core/src/types/operators.tspackages/core/src/types/view-computed.tspackages/generators/src/likec4/operators/expressions.spec.tspackages/generators/src/likec4/operators/expressions.tspackages/generators/src/likec4/schemas/expression.tspackages/language-server/src/__tests__/deployment-views.spec.tspackages/language-server/src/__tests__/views-2.spec.tspackages/language-server/src/formatting/LikeC4Formatter.tspackages/language-server/src/like-c4.langiumpackages/language-server/src/lsp/SemanticTokenProvider.tspackages/language-server/src/model/model-parser-where.tspackages/language-server/src/model/parser/Base.tspackages/likec4/src/LikeC4.spec.tspackages/vscode/likec4.tmLanguage.json
✅ Files skipped from review due to trivial changes (8)
- examples/metadata-views/likec4.config.json
- .changeset/metadata-views-filtering.md
- packages/language-server/src/tests/views-2.spec.ts
- packages/core/src/types/operators.spec.ts
- apps/docs/likec4.tmLanguage.json
- packages/language-server/src/tests/deployment-views.spec.ts
- examples/metadata-views/views.c4
- packages/generators/src/likec4/operators/expressions.spec.ts
🚧 Files skipped from review as they are similar to previous changes (8)
- packages/core/src/model/RelationModel.ts
- packages/language-server/src/model/parser/Base.ts
- apps/playground/likec4.tmLanguage.json
- packages/vscode/likec4.tmLanguage.json
- packages/core/src/model/DeploymentElementModel.ts
- packages/core/src/compute-view/deployment-view/predicates/utils.ts
- packages/core/src/types/operators.ts
- apps/docs/src/content/docs/dsl/Views/predicates.mdx
| export const whereMetadata = z.object({ | ||
| metadata: z.object({ | ||
| key: z.string(), | ||
| value: equalOp(z.string()).optional(), |
There was a problem hiding this comment.
Allow boolean metadata comparisons here.
This PR adds predicates like metadata.critical is true, but whereMetadata.value only validates strings. Generator inputs using boolean metadata filters will be rejected even though the rest of the stack now supports them.
🔧 Proposed fix
export const whereMetadata = z.object({
metadata: z.object({
key: z.string(),
- value: equalOp(z.string()).optional(),
+ value: equalOp(z.union([z.string(), z.boolean()])).optional(),
}),
})🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/generators/src/likec4/schemas/expression.ts` around lines 35 - 38,
The whereMetadata schema currently restricts metadata.value to strings via
equalOp(z.string()).optional(), causing boolean predicates like
metadata.critical is true to be rejected; update the schema so
whereMetadata.value accepts both strings and booleans (e.g. replace
equalOp(z.string()) with equalOp(z.union([z.string(), z.boolean()])) or
equivalent) so the equalOp validation allows boolean metadata comparisons while
keeping the field optional.
| // prodOnly: only prodApi and prodDb have environment=production | ||
| const prodOnlyElements = [...model.view('prodOnly').elements()] | ||
| const prodOnlyIds = prodOnlyElements.map(e => e.id) | ||
| expect(prodOnlyIds).toContain('container.prodApi') | ||
| expect(prodOnlyIds).toContain('container.prodDb') | ||
| expect(prodOnlyIds).not.toContain('container.stagingApi') | ||
|
|
||
| // excludeStaging: all except stagingApi | ||
| const excludeStagingElements = [...model.view('excludeStaging').elements()] | ||
| const excludeStagingIds = excludeStagingElements.map(e => e.id) | ||
| expect(excludeStagingIds).toContain('container.prodApi') | ||
| expect(excludeStagingIds).toContain('container.prodDb') | ||
| expect(excludeStagingIds).not.toContain('container.stagingApi') | ||
|
|
||
| // criticalOnly: only prodApi has critical=true | ||
| const criticalElements = [...model.view('criticalOnly').elements()] | ||
| const criticalIds = criticalElements.map(e => e.id) | ||
| expect(criticalIds).toContain('container.prodApi') | ||
| expect(criticalIds).not.toContain('container.stagingApi') | ||
| expect(criticalIds).not.toContain('container.prodDb') | ||
|
|
||
| // prodNotDb: production elements that are not database kind | ||
| const prodNotDbElements = [...model.view('prodNotDb').elements()] | ||
| const prodNotDbIds = prodNotDbElements.map(e => e.id) | ||
| expect(prodNotDbIds).toContain('container.prodApi') | ||
| expect(prodNotDbIds).not.toContain('container.prodDb') | ||
| expect(prodNotDbIds).not.toContain('container.stagingApi') |
There was a problem hiding this comment.
Make these assertions exact.
Each case only proves a subset via toContain / not.toContain. If metadata filtering starts over-including elements, this test still passes. Compare the full sorted ID list for each view instead.
🧪 Example tightening for one case
- const prodOnlyIds = prodOnlyElements.map(e => e.id)
- expect(prodOnlyIds).toContain('container.prodApi')
- expect(prodOnlyIds).toContain('container.prodDb')
- expect(prodOnlyIds).not.toContain('container.stagingApi')
+ expect(prodOnlyElements.map(e => e.id).sort()).toEqual([
+ 'container.prodApi',
+ 'container.prodDb',
+ ])Based on learnings: Applies to **/*.spec.ts : Aim to cover new features with relevant tests; keep test names descriptive
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/likec4/src/LikeC4.spec.ts` around lines 118 - 144, The current tests
for views (using model.view('prodOnly').elements(), 'excludeStaging',
'criticalOnly', 'prodNotDb') only assert subsets via toContain / not.toContain;
change each block to assert the exact set of element IDs by mapping elements to
ids, sorting, and comparing against the expected sorted array (e.g., assert that
prodOnlyIds sorted equals ['container.prodApi','container.prodDb'] and similarly
tighten excludeStaging, criticalOnly, and prodNotDb to exact expected ID arrays)
so over-inclusion will fail the test.
This PR implements View predicates on metadata mentioned in #2754. I did help myself a lot with Claude given I'm unfamiliar with this codebase.
Checklist
mainbefore creating this PR.I had to regenerate screenshots in E2E tests.
Summary by CodeRabbit
New Features
Editor
Documentation
Tests