Conversation
WalkthroughAdds a row-limit feature to schema checks: new CLI Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
🚥 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
Comment |
SkArchon
requested review from
Aenimus,
JivusAyrus,
StarpTech,
endigma,
jensneuse,
thisisnithin and
wilsonrivera
as code owners
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #2417 +/- ##
==========================================
+ Coverage 36.24% 36.27% +0.03%
==========================================
Files 944 944
Lines 123180 123298 +118
Branches 4927 4944 +17
==========================================
+ Hits 44644 44731 +87
- Misses 76993 77025 +32
+ Partials 1543 1542 -1
🚀 New features to boost your workflow:
|
Router-nonroot image scan passed✅ No security vulnerabilities found in image: |
![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)
cli/src/commands/subgraph/commands/check.ts (1)
56-59: Consider extracting common limit validation logic.The limit validation logic (lines 56-59) is duplicated in
cli/src/commands/graph/monograph/commands/check.ts(lines 38-41). Consider extracting this into a shared utility function to follow the DRY principle.Example shared utility:
// In cli/src/core/validation.ts or similar export function validateCheckLimit(limit: string, maxLimit: number): number { const parsed = Number(limit); if (Number.isNaN(parsed) || parsed <= 0 || parsed > maxLimit) { program.error(pc.red(`The limit must be a valid number between 1 and ${maxLimit}. Received: '${limit}'`)); } return parsed; }Then use it in both check commands:
const limit = validateCheckLimit(options.limit, maxLimit);
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
connect-go/gen/proto/wg/cosmo/platform/v1/platform.pb.gois excluded by!**/*.pb.go,!**/gen/**
📒 Files selected for processing (8)
cli/src/commands/graph/monograph/commands/check.ts(4 hunks)cli/src/commands/subgraph/commands/check.ts(4 hunks)cli/src/handle-check-result.ts(2 hunks)connect/src/wg/cosmo/platform/v1/platform_pb.ts(5 hunks)controlplane/src/core/bufservices/subgraph/checkSubgraphSchema.ts(8 hunks)controlplane/src/core/util.ts(1 hunks)proto/README.md(1 hunks)proto/wg/cosmo/platform/v1/platform.proto(2 hunks)
🧰 Additional context used
🧠 Learnings (7)
📓 Common learnings
Learnt from: JivusAyrus
Repo: wundergraph/cosmo PR: 2331
File: studio/src/components/operations/client-usage-table.tsx:65-65
Timestamp: 2025-11-19T11:26:26.657Z
Learning: In the Cosmo studio codebase (studio/src/components/operations/client-usage-table.tsx), converting BigInt request counts from the API to Number is acceptable because request counts are not expected to exceed JavaScript's safe integer limit (2^53 - 1).
📚 Learning: 2025-08-29T10:10:06.969Z
Learnt from: JivusAyrus
Repo: wundergraph/cosmo PR: 2156
File: studio/src/pages/[organizationSlug]/[namespace]/graph/[slug]/checks/[checkId]/index.tsx:0-0
Timestamp: 2025-08-29T10:10:06.969Z
Learning: The isCheckSuccessful function in both controlplane/src/core/util.ts and studio/src/components/check-badge-icon.tsx includes early exits for linked check validation. It returns false immediately if either isLinkedTrafficCheckFailed or isLinkedPruningCheckFailed is true, which means linked check failures automatically cause the overall check to fail.
Applied to files:
cli/src/handle-check-result.tscontrolplane/src/core/bufservices/subgraph/checkSubgraphSchema.ts
📚 Learning: 2025-09-10T09:59:17.257Z
Learnt from: JivusAyrus
Repo: wundergraph/cosmo PR: 2156
File: controlplane/src/core/bufservices/subgraph/checkSubgraphSchema.ts:417-419
Timestamp: 2025-09-10T09:59:17.257Z
Learning: When skipTrafficCheck is true in schema checks, the traffic inspection loop is completely bypassed (as shown in line 2040 of SubgraphRepository.ts with the continue statement), which means hasClientTraffic will be false. The traffic analysis doesn't run at all when skipTrafficCheck is true, unlike the previous understanding that it always ran to collect usage data.
Applied to files:
cli/src/commands/subgraph/commands/check.ts
📚 Learning: 2025-09-08T20:57:07.946Z
Learnt from: JivusAyrus
Repo: wundergraph/cosmo PR: 2156
File: controlplane/src/core/repositories/SubgraphRepository.ts:1746-1763
Timestamp: 2025-09-08T20:57:07.946Z
Learning: The checkSubgraphSchema.ts file already correctly implements linked subgraph functionality, using byName(linkedSubgraph.name, linkedSubgraph.namespace) to fetch target subgraphs and properly handles parse(newSchemaSDL) for schema building. The implementation doesn't need fixes for byId usage or schema parsing as it's already correct.
Applied to files:
controlplane/src/core/bufservices/subgraph/checkSubgraphSchema.ts
📚 Learning: 2025-08-29T10:28:04.846Z
Learnt from: JivusAyrus
Repo: wundergraph/cosmo PR: 2156
File: controlplane/src/core/repositories/SubgraphRepository.ts:1749-1751
Timestamp: 2025-08-29T10:28:04.846Z
Learning: In the controlplane codebase, authentication and authorization checks (including organization scoping) are handled at the service layer in files like unlinkSubgraph.ts before calling repository methods. Repository methods like unlinkSubgraph() in SubgraphRepository.ts can focus purely on data operations without redundant security checks.
Applied to files:
controlplane/src/core/bufservices/subgraph/checkSubgraphSchema.ts
📚 Learning: 2025-08-31T18:51:32.185Z
Learnt from: JivusAyrus
Repo: wundergraph/cosmo PR: 2156
File: controlplane/src/core/bufservices/check/getCheckSummary.ts:0-0
Timestamp: 2025-08-31T18:51:32.185Z
Learning: In the SchemaCheckRepository.getLinkedSchemaCheck method, organization-level security is implemented through post-query validation by checking `check.subgraphs[0].namespace.organizationId !== organizationId` and returning undefined if the linked check doesn't belong to the caller's organization, preventing cross-tenant data leakage.
Applied to files:
controlplane/src/core/bufservices/subgraph/checkSubgraphSchema.ts
📚 Learning: 2025-09-10T09:52:46.326Z
Learnt from: JivusAyrus
Repo: wundergraph/cosmo PR: 2156
File: controlplane/src/core/bufservices/subgraph/checkSubgraphSchema.ts:417-419
Timestamp: 2025-09-10T09:52:46.326Z
Learning: When skipTrafficCheck is true in schema checks, traffic analysis still runs to collect usage data, but hasClientTraffic can still be true if unsafe client traffic is detected. The difference is that traffic issues don't cause the overall check to fail, but the hasClientTraffic flag itself remains accurate based on the actual traffic analysis results.
Applied to files:
controlplane/src/core/bufservices/subgraph/checkSubgraphSchema.ts
🧬 Code graph analysis (5)
cli/src/handle-check-result.ts (1)
connect/src/wg/cosmo/platform/v1/platform_pb.ts (1)
CheckSubgraphSchemaResponse(2237-2383)
cli/src/commands/subgraph/commands/check.ts (1)
cli/src/handle-check-result.ts (1)
handleCheckResult(9-324)
connect/src/wg/cosmo/platform/v1/platform_pb.ts (1)
connect-go/gen/proto/wg/cosmo/platform/v1/platform.pb.go (3)
SchemaCheckCounts(3256-3269)SchemaCheckCounts(3284-3284)SchemaCheckCounts(3299-3301)
controlplane/src/core/bufservices/subgraph/checkSubgraphSchema.ts (1)
controlplane/src/core/util.ts (2)
clamp(578-580)limitCombinedArrays(487-507)
cli/src/commands/graph/monograph/commands/check.ts (1)
cli/src/handle-check-result.ts (1)
handleCheckResult(9-324)
🪛 GitHub Actions: wgc CI
cli/src/handle-check-result.ts
[error] 264-264: ESLint: 'eqeqeq' rule violated. Expected '!==' and instead saw '!='.
🪛 GitHub Check: build_test_node_matrix (20.x)
cli/src/handle-check-result.ts
[failure] 264-264:
Expected '!==' and instead saw '!='
🪛 GitHub Check: build_test_node_matrix (22.x)
cli/src/handle-check-result.ts
[failure] 264-264:
Expected '!==' and instead saw '!='
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (15)
- GitHub Check: build-router
- GitHub Check: build_push_image (nonroot)
- GitHub Check: integration_test (./events)
- GitHub Check: integration_test (./telemetry)
- GitHub Check: image_scan (nonroot)
- GitHub Check: integration_test (./. ./fuzzquery ./lifecycle ./modules)
- GitHub Check: build_test
- GitHub Check: image_scan
- GitHub Check: build_push_image
- GitHub Check: build_push_image
- GitHub Check: build_test
- GitHub Check: build_test
- GitHub Check: build_push_image
- GitHub Check: Analyze (go)
- GitHub Check: Analyze (javascript-typescript)
🔇 Additional comments (8)
proto/README.md (1)
5-5: Documentation update is accurate and helpful.The
make generatecommand is verified in the Makefile, and both output paths (../connect for TypeScript and ../router/gen for Go) are correctly referenced from the proto/ directory. This documentation addition appropriately guides developers to regenerate protobuf bindings when proto definitions change, which is timely given the new proto fields (limit, counts, SchemaCheckCounts) added in this PR.cli/src/commands/subgraph/commands/check.ts (1)
13-14: LGTM! Limit configuration is well-defined.The maximum limit of 10,000 and default of 50 provide reasonable bounds for schema check output. The implementation correctly integrates with the existing CLI option pattern.
Also applies to: 36-36
cli/src/handle-check-result.ts (1)
9-9: Good implementation of row limiting with user feedback.The addition of the
rowLimitparameter and truncation messaging provides users with clear feedback when results are limited. The logic correctly evaluates grouped counts and provides helpful guidance to view full results in the studio dashboard.Note: This approval is contingent on fixing the
!=vs!==issue on Line 264.Also applies to: 252-268, 277-277, 285-285
proto/wg/cosmo/platform/v1/platform.proto (1)
106-106: LGTM! Protocol buffer definitions are well-structured.The new
limitfield andSchemaCheckCountsmessage properly extend the schema check API with count metadata. The field numbers are properly assigned and types are appropriate for their purposes.Also applies to: 288-300
controlplane/src/core/bufservices/subgraph/checkSubgraphSchema.ts (3)
255-255: Verify limit defaults and maximum values are aligned.There's a discrepancy between the CLI and backend limit constraints:
- CLI default: 50, max: 10,000
- Backend default: 1, max: 100,000
This means the backend would accept limits up to 100,000 even though the CLI caps at 10,000. Consider whether these values should be aligned, or if the backend intentionally allows a higher maximum for other API clients.
Additionally, a default of 1 when no limit is provided seems quite restrictive. Was this intentional, or should it match the CLI default of 50?
291-307: LGTM! Array limiting logic correctly implements grouped behavior.The implementation correctly applies limits:
- Composition errors and warnings are limited separately (they appear in separate CLI tables)
- Breaking/non-breaking changes are grouped (limited together with priority to breaking)
- Lint errors/warnings are grouped (limited together with priority to errors)
- Graph prune errors/warnings are grouped (limited together with priority to errors)
This matches the PR objectives and provides sensible prioritization when limits are exceeded.
310-319: LGTM! Counts object correctly tracks original result sizes.The
countsobject properly captures the total counts fromcheckResultbefore any slicing occurs. This ensures that clients can detect when results have been truncated by comparing counts with the actual array lengths in the response.connect/src/wg/cosmo/platform/v1/platform_pb.ts (1)
1102-1105: LGTM! Generated protobuf code correctly implements the schema check limit and counts.The changes add:
- Optional
limitfield toCheckSubgraphSchemaRequestfor controlling result set size- Optional
countsfield toCheckSubgraphSchemaResponsefor returning total counts- New
SchemaCheckCountsmessage with 8 count fields matching the Go counterpartField numbers are consistent across TypeScript and Go implementations, and all structures follow standard protobuf-ts patterns.
Also applies to: 1124-1124, 2334-2337, 2365-2365, 2385-2462
…d-prune-issues-displayed-on-the
SkArchon
changed the title
…d-prune-issues-displayed-on-the
|
This PR was marked stale due to lack of activity. It will be closed in 14 days. |
…t-the-number-of-lint-and-prune-issues-displayed-on-the
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
controlplane/src/core/util.ts (1)
474-511: LGTM! Consider adding validation for negative limits.The
limitCombinedArraysfunction correctly implements priority-based distribution across multiple arrays. The logic handles edge cases well (empty input, null limit), and the implementation aligns with the documented examples.However, consider adding validation to ensure
limit >= 0when it's not null. A negative limit would cause the function to return empty arrays for all inputs, which might be unexpected behavior.Optional: Add defensive check for negative limits
export function limitCombinedArrays<T>(arrays: T[][], limit: number | null): T[][] { if (arrays.length === 0) { return []; } if (limit == null) { return arrays; } + + // Treat negative limits as zero (return empty arrays) + if (limit < 0) { + return arrays.map(() => []); + } const result: T[][] = []; let remaining = limit;controlplane/src/core/bufservices/subgraph/checkSubgraphSchema.ts (1)
257-258: Consider the edge case wherereq.limitis explicitly set to 0.The current implementation uses a falsy check (
req.limit ?), which means an explicitlimit: 0would be treated the same as "no limit provided" and return all rows. This aligns with the guidance that "if the limit is not passed, return everything," but it might be unexpected if a caller explicitly passes 0 expecting no results.This is likely acceptable behavior (valid limits are 1 to maxRowLimit, everything else means "no limit"), but consider adding a comment to document this design decision.
Optional: Document the limit=0 behavior
- // If req.limit is not provided, we return all rows + // If req.limit is not provided (undefined, null, or 0), we return all rows. + // Valid limit values are clamped to [1, maxRowLimit]. const returnLimit = req.limit ? clamp(req.limit, 1, maxRowLimit) : null;
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
controlplane/src/core/bufservices/subgraph/checkSubgraphSchema.tscontrolplane/src/core/util.ts
🧰 Additional context used
🧠 Learnings (4)
📓 Common learnings
Learnt from: endigma
Repo: wundergraph/cosmo PR: 2009
File: router/pkg/config/config.go:0-0
Timestamp: 2025-07-03T10:33:25.778Z
Learning: The CardinalityLimit field in the Metrics struct (router/pkg/config/config.go) is validated at the JSON schema level in config.schema.json with a minimum value constraint of 1, preventing zero or negative values without requiring runtime validation.
📚 Learning: 2025-09-08T20:57:07.946Z
Learnt from: JivusAyrus
Repo: wundergraph/cosmo PR: 2156
File: controlplane/src/core/repositories/SubgraphRepository.ts:1746-1763
Timestamp: 2025-09-08T20:57:07.946Z
Learning: The checkSubgraphSchema.ts file already correctly implements linked subgraph functionality, using byName(linkedSubgraph.name, linkedSubgraph.namespace) to fetch target subgraphs and properly handles parse(newSchemaSDL) for schema building. The implementation doesn't need fixes for byId usage or schema parsing as it's already correct.
Applied to files:
controlplane/src/core/bufservices/subgraph/checkSubgraphSchema.ts
📚 Learning: 2025-09-10T09:52:46.326Z
Learnt from: JivusAyrus
Repo: wundergraph/cosmo PR: 2156
File: controlplane/src/core/bufservices/subgraph/checkSubgraphSchema.ts:417-419
Timestamp: 2025-09-10T09:52:46.326Z
Learning: When skipTrafficCheck is true in schema checks, traffic analysis still runs to collect usage data, but hasClientTraffic can still be true if unsafe client traffic is detected. The difference is that traffic issues don't cause the overall check to fail, but the hasClientTraffic flag itself remains accurate based on the actual traffic analysis results.
Applied to files:
controlplane/src/core/bufservices/subgraph/checkSubgraphSchema.ts
📚 Learning: 2025-08-29T10:10:06.969Z
Learnt from: JivusAyrus
Repo: wundergraph/cosmo PR: 2156
File: studio/src/pages/[organizationSlug]/[namespace]/graph/[slug]/checks/[checkId]/index.tsx:0-0
Timestamp: 2025-08-29T10:10:06.969Z
Learning: The isCheckSuccessful function in both controlplane/src/core/util.ts and studio/src/components/check-badge-icon.tsx includes early exits for linked check validation. It returns false immediately if either isLinkedTrafficCheckFailed or isLinkedPruningCheckFailed is true, which means linked check failures automatically cause the overall check to fail.
Applied to files:
controlplane/src/core/bufservices/subgraph/checkSubgraphSchema.ts
🧬 Code graph analysis (1)
controlplane/src/core/bufservices/subgraph/checkSubgraphSchema.ts (1)
controlplane/src/core/util.ts (2)
clamp(582-584)limitCombinedArrays(487-511)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (15)
- GitHub Check: build-router
- GitHub Check: Analyze (javascript-typescript)
- GitHub Check: Analyze (go)
- GitHub Check: build_push_image
- GitHub Check: build_test
- GitHub Check: image_scan (nonroot)
- GitHub Check: build_push_image (nonroot)
- GitHub Check: integration_test (./. ./fuzzquery ./lifecycle ./modules)
- GitHub Check: integration_test (./telemetry)
- GitHub Check: build_test
- GitHub Check: build_test
- GitHub Check: build_push_image
- GitHub Check: integration_test (./events)
- GitHub Check: image_scan
- GitHub Check: build_push_image
🔇 Additional comments (5)
controlplane/src/core/bufservices/subgraph/checkSubgraphSchema.ts (5)
31-31: LGTM! Reasonable upper bound.The
maxRowLimitconstant of 100,000 provides a sensible upper bound for result limiting, protecting against excessive memory usage while allowing sufficiently large result sets for most use cases.
294-297: LGTM! Correct independent limiting.Composition errors and warnings are correctly limited independently (not grouped), which aligns with the PR objective that they are displayed in separate CLI tables.
299-312: LGTM! Grouped limiting correctly applied.The grouped limiting for breaking/non-breaking changes, lint errors/warnings, and graph prune errors/warnings correctly uses
limitCombinedArrayswith appropriate priority ordering (errors before warnings). This aligns with the PR objective to treat these pairs as combined counts for display purposes.
314-324: Excellent design! Counts capture original totals.The
countsobject correctly captures the total counts from the originalcheckResultbefore limiting is applied. This is essential for clients to determine whether results were truncated and to display accurate totals (e.g., "Showing 50 of 150 errors"). Well done!
345-345: LGTM! Consistent counts propagation.The
countsobject is correctly propagated through all return paths afterperformSchemaCheckis called, ensuring clients always receive count metadata when a schema check is performed. Early validation errors (before the check runs) appropriately omit counts.Also applies to: 375-375, 406-406, 468-468, 517-517
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In @cli/src/handle-check-result.ts:
- Around line 261-270: The code currently overwrites the newline added to
moreEntriesAvailableMessage because it uses assignment when setting the red
message; update the logic in the hasExceeded branch (referencing hasExceeded,
studioCheckDestination, moreEntriesAvailableMessage, rowLimit, pc.red) to
concatenate rather than reassign so the `\n\n` is preserved—e.g., append the red
truncated message with `+=` or build the full message first (including the `They
can be viewed...` clause when studioCheckDestination !== '') and then apply
pc.red to the complete string so no parts are lost.
🧹 Nitpick comments (5)
proto/wg/cosmo/platform/v1/platform.proto (2)
106-106: Consider adding field documentation and validation constraints.The
limitfield lacks documentation explaining its purpose, valid range, and behavior when exceeded. While the service layer clamps negative/zero values, proto-level constraints (or at least comments) would help API consumers understand expectations upfront.📝 Suggested documentation
optional bool disable_resolvability_validation = 9; + // limit restricts the maximum number of entries returned per category (e.g., lint issues, schema changes). + // Valid range: 1 to 100,000. Defaults to unlimited when not specified. optional int32 limit = 10;
762-762: Same validation and documentation concern as Line 106.The
limitfield here has the same lack of documentation and validation constraints. Consider applying the same documentation improvements suggested for theCheckSubgraphSchemaRequest.limitfield.controlplane/src/core/bufservices/federated-graph/checkFederatedGraph.ts (3)
28-28: Document the maximum row limit constant.The
maxRowLimitconstant lacks documentation explaining why 100,000 is the chosen maximum. Consider adding a comment describing the rationale (e.g., performance considerations, UI constraints).📝 Suggested documentation
+// Maximum number of rows that can be returned per category when a limit is specified. +// This cap prevents performance degradation from excessively large result sets. const maxRowLimit = 100_000;
116-117: Consider explicit validation for invalid limit values.The code silently corrects invalid
limitvalues (negative or zero) to 1 viaclamp. Users submitting invalid values won't receive feedback that their input was adjusted. Consider adding explicit validation that returns an error response for invalid limits.✅ Proposed validation
+ if (req.limit !== undefined && req.limit <= 0) { + return { + response: { + code: EnumStatusCode.ERR_INVALID_LIMIT, + details: `Limit must be a positive integer (provided: ${req.limit})`, + }, + compositionErrors: [], + subgraphs: [], + compositionWarnings: [], + }; + } + // If req.limit is not provided, we return all rows - const returnLimit = req.limit ? clamp(req.limit, 1, maxRowLimit) : null; + const returnLimit = req.limit ? Math.min(req.limit, maxRowLimit) : null;
119-128: Verify the need for unused count fields.The
countsobject initializeslintWarnings,lintErrors,breakingChanges,nonBreakingChanges,graphPruneErrors, andgraphPruneWarningsfields that are never populated in this federated graph check flow. While maintaining a consistent response shape withcheckSubgraphSchemamay be intentional, returning these fields with zero values could confuse API consumers.
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
connect-go/gen/proto/wg/cosmo/platform/v1/platform.pb.gois excluded by!**/*.pb.go,!**/gen/**
📒 Files selected for processing (8)
cli/src/commands/graph/federated-graph/commands/check.tscli/src/commands/graph/monograph/commands/check.tscli/src/commands/subgraph/commands/check.tscli/src/constants.tscli/src/handle-check-result.tsconnect/src/wg/cosmo/platform/v1/platform_pb.tscontrolplane/src/core/bufservices/federated-graph/checkFederatedGraph.tsproto/wg/cosmo/platform/v1/platform.proto
🚧 Files skipped from review as they are similar to previous changes (1)
- cli/src/commands/graph/monograph/commands/check.ts
🧰 Additional context used
🧠 Learnings (4)
📓 Common learnings
Learnt from: endigma
Repo: wundergraph/cosmo PR: 2009
File: router/pkg/config/config.go:0-0
Timestamp: 2025-07-03T10:33:25.778Z
Learning: The CardinalityLimit field in the Metrics struct (router/pkg/config/config.go) is validated at the JSON schema level in config.schema.json with a minimum value constraint of 1, preventing zero or negative values without requiring runtime validation.
📚 Learning: 2025-08-28T09:17:49.477Z
Learnt from: endigma
Repo: wundergraph/cosmo PR: 2141
File: router-tests/http_subscriptions_test.go:17-55
Timestamp: 2025-08-28T09:17:49.477Z
Learning: The Cosmo router uses a custom, intentionally rigid multipart implementation for GraphQL subscriptions. The multipart parsing in test files should remain strict and not be made more tolerant, as this rigidity is by design.
Applied to files:
cli/src/constants.ts
📚 Learning: 2025-08-29T10:10:06.969Z
Learnt from: JivusAyrus
Repo: wundergraph/cosmo PR: 2156
File: studio/src/pages/[organizationSlug]/[namespace]/graph/[slug]/checks/[checkId]/index.tsx:0-0
Timestamp: 2025-08-29T10:10:06.969Z
Learning: The isCheckSuccessful function in both controlplane/src/core/util.ts and studio/src/components/check-badge-icon.tsx includes early exits for linked check validation. It returns false immediately if either isLinkedTrafficCheckFailed or isLinkedPruningCheckFailed is true, which means linked check failures automatically cause the overall check to fail.
Applied to files:
cli/src/handle-check-result.ts
📚 Learning: 2025-09-10T09:59:17.257Z
Learnt from: JivusAyrus
Repo: wundergraph/cosmo PR: 2156
File: controlplane/src/core/bufservices/subgraph/checkSubgraphSchema.ts:417-419
Timestamp: 2025-09-10T09:59:17.257Z
Learning: When skipTrafficCheck is true in schema checks, the traffic inspection loop is completely bypassed (as shown in line 2040 of SubgraphRepository.ts with the continue statement), which means hasClientTraffic will be false. The traffic analysis doesn't run at all when skipTrafficCheck is true, unlike the previous understanding that it always ran to collect usage data.
Applied to files:
cli/src/commands/subgraph/commands/check.ts
🧬 Code graph analysis (5)
connect/src/wg/cosmo/platform/v1/platform_pb.ts (1)
connect-go/gen/proto/wg/cosmo/platform/v1/platform.pb.go (3)
SchemaCheckCounts(3256-3269)SchemaCheckCounts(3284-3284)SchemaCheckCounts(3299-3301)
cli/src/commands/graph/federated-graph/commands/check.ts (2)
cli/src/constants.ts (1)
limitMaxValue(7-7)cli/src/commands/graph/monograph/commands/check.ts (1)
opts(13-90)
cli/src/handle-check-result.ts (2)
connect/src/wg/cosmo/platform/v1/platform_pb.ts (1)
CheckSubgraphSchemaResponse(2237-2383)connect-go/gen/proto/wg/cosmo/platform/v1/platform.pb.go (3)
CheckSubgraphSchemaResponse(3062-3089)CheckSubgraphSchemaResponse(3104-3104)CheckSubgraphSchemaResponse(3119-3121)
cli/src/commands/subgraph/commands/check.ts (1)
cli/src/handle-check-result.ts (1)
handleCheckResult(9-328)
controlplane/src/core/bufservices/federated-graph/checkFederatedGraph.ts (1)
controlplane/src/core/util.ts (1)
clamp(582-584)
🔇 Additional comments (14)
proto/wg/cosmo/platform/v1/platform.proto (1)
291-300: LGTM!The
SchemaCheckCountsmessage structure is well-designed with clear field names and appropriate types. This enables the API to communicate both the actual entries returned and the total counts, supporting proper truncation notification in the CLI.controlplane/src/core/bufservices/federated-graph/checkFederatedGraph.ts (1)
133-141: Verify per-category limit behavior matches requirements.The limit is applied separately to warnings (line 133) and errors (line 147), meaning a
limitof 10 could return up to 20 total items (10 warnings + 10 errors). The PR objectives mention that "compositionErrors and compositionWarnings are not grouped because they are displayed in two separate CLI tables," which suggests this behavior is intentional. However, it differs from typical pagination where a limit applies to the total result set.Please confirm this per-category limiting aligns with the intended UX. If users expect
--limit 10to return at most 10 total items across all categories, consider using thelimitCombinedArraysutility mentioned in the related code snippets.connect/src/wg/cosmo/platform/v1/platform_pb.ts (5)
1102-1124: LGTM — Correctly generatedlimitfield for request limiting.The field definition aligns with the Go version's pattern where
SchemaCheckCountsusesint32protobuf types with proper field numbering. The optionalint32is appropriate for a limit parameter that should default to a server-defined value when not provided.
2334-2365: LGTM —countsfield correctly added to response.The optional message field for
SchemaCheckCountsfollows the standard protobuf generation pattern and field number 19 is sequential with the existing fields.
2385-2462: LGTM —SchemaCheckCountsclass is correctly generated and consistent with Go version.The TypeScript class matches the Go struct definition with identical field names (
LintWarnings,LintErrors,BreakingChanges,NonBreakingChanges,CompositionErrors,CompositionWarnings,GraphPruneErrors,GraphPruneWarnings) and field numbers (1-8).The class follows standard
@bufbuild/protobufpatterns with proper static helpers and field list definitions.
5859-5876: LGTM —limitfield correctly added toCheckFederatedGraphRequest.Consistent with the
CheckSubgraphSchemaRequestpattern, using optionalint32for the limit parameter.
5920-5937: LGTM —countsfield correctly added toCheckFederatedGraphResponse.The response now includes the same
SchemaCheckCountsmessage type asCheckSubgraphSchemaResponse, providing consistent count reporting across both API endpoints.cli/src/constants.ts (1)
6-7: LGTM!Good use of a centralized constant for the maximum limit value with numeric separators for readability.
cli/src/commands/graph/federated-graph/commands/check.ts (3)
28-28: LGTM!The limit option is consistently defined with the same default value ('50') as the other check commands.
33-36: LGTM!Limit validation is correctly implemented and consistent with the other check commands.
107-109: LGTM!Truncation messaging correctly checks composition errors and warnings individually (since they're displayed in separate tables), consistent with the PR's design.
Also applies to: 127-129
cli/src/handle-check-result.ts (1)
9-9: LGTM!Function signature correctly updated to accept the row limit parameter.
cli/src/commands/subgraph/commands/check.ts (2)
12-12: LGTM!Import and option definition are consistent with the other check commands.
Also applies to: 35-35
55-58: LGTM!Limit validation, API payload, and
handleCheckResultcall are correctly implemented and consistent with the other check commands.Also applies to: 84-84, 92-92
…t-the-number-of-lint-and-prune-issues-displayed-on-the
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (3)
proto/wg/cosmo/platform/v1/platform.proto (3)
106-106: Consider adding a documentation comment for the limit field.The
limitfield would benefit from a comment explaining what it constrains (e.g., "Maximum number of entries to return across diagnostic arrays") and noting that default and maximum values are enforced server-side. This improves API discoverability.📝 Suggested documentation
optional bool disable_resolvability_validation = 9; + // Maximum number of diagnostic entries to return. Default and maximum limits are enforced server-side. + // Applies across lintWarnings, lintErrors, breakingChanges, nonBreakingChanges, graphPruneErrors, and graphPruneWarnings. optional int32 limit = 10;
291-300: Add documentation to clarify the purpose of SchemaCheckCounts.The
SchemaCheckCountsmessage would benefit from a comment explaining that these fields represent total counts (before any limiting), allowing clients to detect when results have been truncated and inform users accordingly.📝 Suggested documentation
+// SchemaCheckCounts contains the total count of diagnostic issues found during schema checks. +// These counts represent all issues detected, regardless of the limit parameter. +// Clients can compare these totals with the number of items returned to detect truncation. message SchemaCheckCounts{ int32 lintWarnings = 1; int32 lintErrors = 2;
762-762: Consider adding a documentation comment for the limit field.Similar to
CheckSubgraphSchemaRequest.limit, this field would benefit from documentation explaining its purpose and constraints. Consistent documentation across both request types improves API usability.📝 Suggested documentation
optional bool disable_resolvability_validation = 4; + // Maximum number of diagnostic entries to return. Default and maximum limits are enforced server-side. + // Applies across composition errors and warnings in the response. optional int32 limit = 5;
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
connect-go/gen/proto/wg/cosmo/platform/v1/platform.pb.gois excluded by!**/*.pb.go,!**/gen/**
📒 Files selected for processing (3)
cli/src/handle-check-result.tsconnect/src/wg/cosmo/platform/v1/platform_pb.tsproto/wg/cosmo/platform/v1/platform.proto
🧰 Additional context used
🧠 Learnings (2)
📚 Learning: 2025-08-29T10:10:06.969Z
Learnt from: JivusAyrus
Repo: wundergraph/cosmo PR: 2156
File: studio/src/pages/[organizationSlug]/[namespace]/graph/[slug]/checks/[checkId]/index.tsx:0-0
Timestamp: 2025-08-29T10:10:06.969Z
Learning: The isCheckSuccessful function in both controlplane/src/core/util.ts and studio/src/components/check-badge-icon.tsx includes early exits for linked check validation. It returns false immediately if either isLinkedTrafficCheckFailed or isLinkedPruningCheckFailed is true, which means linked check failures automatically cause the overall check to fail.
Applied to files:
cli/src/handle-check-result.ts
📚 Learning: 2026-01-05T11:53:49.921Z
Learnt from: asoorm
Repo: wundergraph/cosmo PR: 2438
File: router-tests/go.mod:15-15
Timestamp: 2026-01-05T11:53:49.921Z
Learning: For mcp-go dependency in wundergraph/cosmo, patch releases may not have formal release notes published. Tag diffs should be reviewed to verify patch-level changes contain no API surface changes or breaking behavior before adopting new patch versions.
Applied to files:
connect/src/wg/cosmo/platform/v1/platform_pb.ts
🧬 Code graph analysis (2)
cli/src/handle-check-result.ts (2)
connect/src/wg/cosmo/platform/v1/platform_pb.ts (1)
CheckSubgraphSchemaResponse(2237-2383)connect-go/gen/proto/wg/cosmo/platform/v1/platform.pb.go (3)
CheckSubgraphSchemaResponse(3062-3089)CheckSubgraphSchemaResponse(3104-3104)CheckSubgraphSchemaResponse(3119-3121)
connect/src/wg/cosmo/platform/v1/platform_pb.ts (1)
connect-go/gen/proto/wg/cosmo/platform/v1/platform.pb.go (3)
SchemaCheckCounts(3256-3269)SchemaCheckCounts(3284-3284)SchemaCheckCounts(3299-3301)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (15)
- GitHub Check: build-router
- GitHub Check: build_push_image
- GitHub Check: build_push_image (nonroot)
- GitHub Check: image_scan (nonroot)
- GitHub Check: integration_test (./telemetry)
- GitHub Check: image_scan
- GitHub Check: integration_test (./. ./fuzzquery ./lifecycle ./modules)
- GitHub Check: integration_test (./events)
- GitHub Check: build_test
- GitHub Check: build_test
- GitHub Check: build_push_image
- GitHub Check: build_test
- GitHub Check: build_push_image
- GitHub Check: Analyze (javascript-typescript)
- GitHub Check: Analyze (go)
🔇 Additional comments (10)
proto/wg/cosmo/platform/v1/platform.proto (2)
770-770: LGTM - Consistent with CheckSubgraphSchemaResponse.The addition of the
countsfield is well-structured and maintains consistency with the parallel change inCheckSubgraphSchemaResponse. The field tag is correctly sequenced.
106-106: Well-structured proto additions for limiting schema check results.The changes correctly introduce the limit parameter and counts tracking across both subgraph and federated graph schema checks. Field tag numbering is sequential and correct, types are appropriate, and the changes are backward compatible. The main enhancement opportunity is adding documentation comments to improve API discoverability.
Also applies to: 288-300, 762-762, 770-770
connect/src/wg/cosmo/platform/v1/platform_pb.ts (5)
1102-1105: LGTM:limitfield properly added to CheckSubgraphSchemaRequest.The generated code correctly adds the optional
limitfield (field #10) with appropriate type mapping (int32 → number).Also applies to: 1124-1124
2334-2337: LGTM:countsfield properly added to CheckSubgraphSchemaResponse.The generated code correctly adds the optional
countsfield (field #19) referencing the SchemaCheckCounts message type.Also applies to: 2365-2365
2385-2462: LGTM: SchemaCheckCounts message correctly generated.The SchemaCheckCounts class is properly defined with all 8 count fields (lintWarnings, lintErrors, breakingChanges, nonBreakingChanges, compositionErrors, compositionWarnings, graphPruneErrors, graphPruneWarnings) and standard protobuf methods. The implementation matches the Go version shown in the relevant snippets.
5859-5862: LGTM:limitfield properly added to CheckFederatedGraphRequest.The generated code correctly adds the optional
limitfield (field #5) consistent with the implementation in CheckSubgraphSchemaRequest.Also applies to: 5876-5876
5920-5923: LGTM:countsfield properly added to CheckFederatedGraphResponse.The generated code correctly adds the optional
countsfield (field #5) consistent with the implementation in CheckSubgraphSchemaResponse.Also applies to: 5937-5937
cli/src/handle-check-result.ts (3)
9-9: LGTM: Function signature updated correctly.The
rowLimitparameter is appropriately typed and enables the new truncation feature.
252-272: LGTM: Truncation logic implemented correctly.The implementation properly:
- Guards against undefined
resp.countsbefore access- Groups counts consistently with the PR description (lint warnings+errors, breaking+non-breaking changes, prune errors+warnings, and separate composition counts)
- Uses strict inequality (
>) so results at exactly the limit don't trigger truncation- Conditionally formats the message with appropriate spacing and dashboard guidance based on whether a studio link exists
- Applies the previously identified fixes (strict equality on lines 262, 268 and concatenation with
+=on line 265)
281-281: LGTM: Truncation message integrated correctly in both output paths.The message is appropriately appended to both success (line 281) and failure (line 289) outputs, ensuring users are notified of truncation regardless of check outcome.
Also applies to: 289-289
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
controlplane/src/core/bufservices/federated-graph/checkFederatedGraph.ts (1)
28-28: Extract maxRowLimit to a shared constant.The
maxRowLimitconstant is duplicated in bothcheckFederatedGraph.ts(line 28) andcheckSubgraphSchema.ts(line 31). Extract this to a shared location (e.g.,util.tsor a dedicated constants file) to ensure consistency and avoid potential divergence.♻️ Proposed refactor
In
controlplane/src/core/util.ts, add:export const MAX_SCHEMA_CHECK_ROW_LIMIT = 100_000;Then update both files to import and use it:
-const maxRowLimit = 100_000; +import { MAX_SCHEMA_CHECK_ROW_LIMIT } from '../../util.js';-const returnLimit = req.limit === undefined ? null : clamp(req.limit, 1, maxRowLimit); +const returnLimit = req.limit === undefined ? null : clamp(req.limit, 1, MAX_SCHEMA_CHECK_ROW_LIMIT);
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
controlplane/src/core/bufservices/federated-graph/checkFederatedGraph.tscontrolplane/src/core/bufservices/subgraph/checkSubgraphSchema.tscontrolplane/test/check-federated-graph.test.tscontrolplane/test/check-subgraph-schema.test.ts
🧰 Additional context used
🧠 Learnings (7)
📚 Learning: 2025-09-08T20:57:07.946Z
Learnt from: JivusAyrus
Repo: wundergraph/cosmo PR: 2156
File: controlplane/src/core/repositories/SubgraphRepository.ts:1746-1763
Timestamp: 2025-09-08T20:57:07.946Z
Learning: The checkSubgraphSchema.ts file already correctly implements linked subgraph functionality, using byName(linkedSubgraph.name, linkedSubgraph.namespace) to fetch target subgraphs and properly handles parse(newSchemaSDL) for schema building. The implementation doesn't need fixes for byId usage or schema parsing as it's already correct.
Applied to files:
controlplane/test/check-federated-graph.test.tscontrolplane/src/core/bufservices/subgraph/checkSubgraphSchema.tscontrolplane/test/check-subgraph-schema.test.ts
📚 Learning: 2025-10-17T16:35:24.723Z
Learnt from: Aenimus
Repo: wundergraph/cosmo PR: 2287
File: composition/tests/v1/federation-factory.test.ts:569-583
Timestamp: 2025-10-17T16:35:24.723Z
Learning: In the wundergraph/cosmo repository's Vitest test suite, `toHaveLength()` works with Map objects and `expect(map.has(key))` without an explicit matcher correctly asserts the boolean result in the test assertions for `composition/tests/v1/federation-factory.test.ts`.
Applied to files:
controlplane/test/check-federated-graph.test.ts
📚 Learning: 2025-09-10T09:52:46.326Z
Learnt from: JivusAyrus
Repo: wundergraph/cosmo PR: 2156
File: controlplane/src/core/bufservices/subgraph/checkSubgraphSchema.ts:417-419
Timestamp: 2025-09-10T09:52:46.326Z
Learning: When skipTrafficCheck is true in schema checks, traffic analysis still runs to collect usage data, but hasClientTraffic can still be true if unsafe client traffic is detected. The difference is that traffic issues don't cause the overall check to fail, but the hasClientTraffic flag itself remains accurate based on the actual traffic analysis results.
Applied to files:
controlplane/src/core/bufservices/subgraph/checkSubgraphSchema.ts
📚 Learning: 2025-08-29T10:10:06.969Z
Learnt from: JivusAyrus
Repo: wundergraph/cosmo PR: 2156
File: studio/src/pages/[organizationSlug]/[namespace]/graph/[slug]/checks/[checkId]/index.tsx:0-0
Timestamp: 2025-08-29T10:10:06.969Z
Learning: The isCheckSuccessful function in both controlplane/src/core/util.ts and studio/src/components/check-badge-icon.tsx includes early exits for linked check validation. It returns false immediately if either isLinkedTrafficCheckFailed or isLinkedPruningCheckFailed is true, which means linked check failures automatically cause the overall check to fail.
Applied to files:
controlplane/src/core/bufservices/subgraph/checkSubgraphSchema.ts
📚 Learning: 2025-08-29T10:28:04.846Z
Learnt from: JivusAyrus
Repo: wundergraph/cosmo PR: 2156
File: controlplane/src/core/repositories/SubgraphRepository.ts:1749-1751
Timestamp: 2025-08-29T10:28:04.846Z
Learning: In the controlplane codebase, authentication and authorization checks (including organization scoping) are handled at the service layer in files like unlinkSubgraph.ts before calling repository methods. Repository methods like unlinkSubgraph() in SubgraphRepository.ts can focus purely on data operations without redundant security checks.
Applied to files:
controlplane/test/check-subgraph-schema.test.ts
📚 Learning: 2025-08-31T18:51:32.185Z
Learnt from: JivusAyrus
Repo: wundergraph/cosmo PR: 2156
File: controlplane/src/core/bufservices/check/getCheckSummary.ts:0-0
Timestamp: 2025-08-31T18:51:32.185Z
Learning: In the SchemaCheckRepository.getLinkedSchemaCheck method, organization-level security is implemented through post-query validation by checking `check.subgraphs[0].namespace.organizationId !== organizationId` and returning undefined if the linked check doesn't belong to the caller's organization, preventing cross-tenant data leakage.
Applied to files:
controlplane/test/check-subgraph-schema.test.ts
📚 Learning: 2025-09-10T09:59:17.257Z
Learnt from: JivusAyrus
Repo: wundergraph/cosmo PR: 2156
File: controlplane/src/core/bufservices/subgraph/checkSubgraphSchema.ts:417-419
Timestamp: 2025-09-10T09:59:17.257Z
Learning: When skipTrafficCheck is true in schema checks, the traffic inspection loop is completely bypassed (as shown in line 2040 of SubgraphRepository.ts with the continue statement), which means hasClientTraffic will be false. The traffic analysis doesn't run at all when skipTrafficCheck is true, unlike the previous understanding that it always ran to collect usage data.
Applied to files:
controlplane/test/check-subgraph-schema.test.ts
🧬 Code graph analysis (3)
controlplane/test/check-federated-graph.test.ts (3)
controlplane/test/test-util.ts (2)
SetupTest(66-418)DEFAULT_NAMESPACE(52-52)controlplane/src/core/test-util.ts (2)
genID(53-55)genUniqueLabel(57-59)shared/src/utils/util.ts (1)
joinLabel(17-19)
controlplane/src/core/bufservices/federated-graph/checkFederatedGraph.ts (1)
controlplane/src/core/util.ts (1)
clamp(582-584)
controlplane/src/core/bufservices/subgraph/checkSubgraphSchema.ts (1)
controlplane/src/core/util.ts (2)
clamp(582-584)limitCombinedArrays(487-511)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (15)
- GitHub Check: build-router
- GitHub Check: build_test
- GitHub Check: build_push_image
- GitHub Check: Analyze (javascript-typescript)
- GitHub Check: Analyze (go)
- GitHub Check: integration_test (./telemetry)
- GitHub Check: image_scan
- GitHub Check: build_test
- GitHub Check: integration_test (./. ./fuzzquery ./lifecycle ./modules)
- GitHub Check: integration_test (./events)
- GitHub Check: build_push_image
- GitHub Check: build_push_image (nonroot)
- GitHub Check: build_test
- GitHub Check: image_scan (nonroot)
- GitHub Check: build_push_image
🔇 Additional comments (10)
controlplane/src/core/bufservices/federated-graph/checkFederatedGraph.ts (3)
116-117: LGTM! Correct limit handling.The returnLimit calculation properly defaults to returning all rows when no limit is provided (null), and clamps provided values between 1 and maxRowLimit. This aligns with the expected behavior noted in past reviews.
119-128: Verify counts object structure for federated graph checks.The counts object includes fields like
lintWarnings,lintErrors,breakingChanges,nonBreakingChanges,graphPruneErrors, andgraphPruneWarnings, butcheckFederatedGraphonly populatescompositionErrorsandcompositionWarnings. All other fields remain 0.While this maintains consistency with the
checkSubgraphSchemaresponse structure, confirm this is the intended design. If these fields will always be zero for federated graph checks, consider documenting this behavior or using a more specific response type.Based on learnings, the API continues to return individual count attributes in SchemaCheckCounts. Verify that clients (CLI) handle these zero-valued fields appropriately.
131-177: LGTM! Correct counts and limit handling.The implementation correctly:
- Sets counts to reflect the full totals before limiting
- Slices the returned arrays based on
returnLimit- Propagates counts in both error and success response branches
- Handles composition errors and warnings separately (as intended per the PR design)
controlplane/src/core/bufservices/subgraph/checkSubgraphSchema.ts (5)
257-258: LGTM! Consistent limit handling.The returnLimit calculation matches the pattern in
checkFederatedGraph.tsand correctly implements the expected behavior.
260-297: LGTM! Correct extraction and slicing pattern.The two-step approach (capture full checkResult, then extract/slice) is clean and allows proper separation of:
- Original array lengths for counts
- Limited arrays for response
Composition errors and warnings are correctly sliced separately rather than combined, matching the PR design that states they "appear in two separate CLI tables."
299-312: LGTM! Correct paired array limiting.The use of
limitCombinedArrayscorrectly groups related arrays (breaking/non-breaking changes, lint errors/warnings, prune errors/warnings) and distributes the limit across them. This aligns with the PR design where these pairs are treated as grouped.
315-324: LGTM! Counts correctly reflect totals.The counts object properly captures the full totals from the original checkResult arrays before limiting. This ensures clients can see how many items were truncated.
345-345: LGTM! Counts consistently propagated across all response paths.The counts object is included in all response branches (non-OK, linked subgraph errors, and success), ensuring clients always receive total counts regardless of the check outcome. This is good for API consistency.
Also applies to: 375-375, 406-406, 468-468, 517-517
controlplane/test/check-subgraph-schema.test.ts (1)
1225-1513: LGTM! Comprehensive test coverage for limit parameter.The test suite thoroughly validates the limit functionality:
- Default behavior (no limit returns all)
- Limit enforcement and distribution across combined arrays
- Boundary conditions (0, values exceeding max)
- Separate handling of composition errors
- Priority of breaking changes over non-breaking
- Counts accuracy (always reflecting totals)
- Counts present even when no changes exist
The tests follow the existing patterns in the file and provide good coverage.
controlplane/test/check-federated-graph.test.ts (1)
526-832: LGTM! Appropriate test coverage for federated graph checks.The test suite provides good coverage for federated graph check limits:
- Composition error limiting
- Composition warning limiting (using
@deprecatedto generate warnings)- Counts consistency in both success and failure scenarios
- Boundary conditions (0, excessive values)
- Verification that counts reflect totals while returned arrays respect limits
The scope is appropriate since federated graph checks only perform composition validation (no breaking changes, lint, or prune checks).
SkArchon
deleted the
milinda/eng-8620-limit-the-number-of-lint-and-prune-issues-displayed-on-the
branch
2 participants
This PR does the following
Introduces a limit parameter for schema checks to pass in a limit, we still do use a default limit if a value was not passed in, we also set a max number.
When determining if the limit was exceeded, we treat the following values as one grouping
We however DO NOT treat
compositionErrorsandcompositionWarningsas one entry because they are displayed in two tables on the CLI at the moment.However when returning the counts, we return values for every individual attribute, which can be found in the
SchemaCheckCountsdefinition. This includes for examplelintWarningsandlintErrorsbeing separate attributes, as they are part of the API, which would be harder to change later if we addlintWarningsAndErrorsand want to break it into two values.The CLI also has a new option for changing the limit with a default value, if there are any truncated entries we mention to the user in the CLI output and ask the user to refer the studio dashboard.
Note: This PR does not introduce pagination to studio.
Summary by CodeRabbit
New Features
Tests
Documentation
✏️ Tip: You can customize this high-level summary in your review settings.
Checklist