fix(schema): stop commas truncating jsonschema description tags#6036
fix(schema): stop commas truncating jsonschema description tags#6036devantler wants to merge 11 commits into
Conversation
jsonschema struct-tag options are comma-separated, so a raw comma inside description= silently cut four field descriptions in the published schema, the generated config reference (breaking its Markdown tables), and the TS types. Reword the four comma-free, regenerate all four artifacts, and add a source-scan regression test that fails on any severed tag piece.
@coderabbitai review |
|
✅ Action performedReview finished.
|
|
Warning Review limit reachedYou’ve reached a temporary PR review limit under our Fair Usage Limits Policy. Next review available in: 42 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (7)
📝 WalkthroughWalkthroughMigrated JSON schema field descriptions across cluster API types to dedicated 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
Comment |
✅MegaLinter analysis: Success✅ Linters with no issuesactionlint, bash-exec, git_diff, hadolint, jscpd, jsonlint, lychee, markdown-table-formatter, markdownlint, prettier, prettier, shellcheck, shfmt, stylelint, syft, trivy-sbom, trufflehog, v8r, v8r, yamllint Notices📣 MegaLinter 9.5.0 is out! Discover the new features and security recommendations in the release announcement. (Skip this info by defining See detailed reports in MegaLinter artifacts
|
Code Coverage OverviewLanguages: Go Go / code-coverage/goThe overall coverage in the branch remains at 66%, unchanged from the branch. Show a code coverage summary of the most impacted files.
Updated |
The jsonschema struct tag's options are comma-separated, so a comma inside an inline description= silently truncates the published schema, the generated configuration reference, and the TS types (#6035). Migrate every inline description= (30 fields across 5 files) to the dedicated jsonschema_description tag, which has no option grammar — the original prose, commas included, now survives verbatim. The tag-guard test now forbids inline description= so the class cannot recur, and all four previously-truncated descriptions are restored in the regenerated artifacts. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@coderabbitai full review |
|
✅ Action performedFull review finished. You're currently rate limited under our Fair Usage Limits Policy. Your recent PR review activity is in the 95th percentile or higher among CodeRabbit users, so adaptive limits apply. Your next review will be available in 41 minutes. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@docs/src/content/docs/configuration/declarative-configuration.mdx`:
- Line 417: The `expander` field is documented only as `[]enum` even though the
schema accepts both a scalar enum and an enum array. Update the source or
generator responsible for the configuration table to render this type as `enum |
[]enum`, then regenerate the documentation with `go generate ./docs/...`; do not
edit the generated MDX directly.
In `@pkg/apis/cluster/v1alpha1/autoscaler.go`:
- Line 33: Update the jsonschema_description on AutoscalerSpec.MaxNodesTotal to
replace the fragment “Should be <= serverLimit” with “The value should be <=
serverLimit”, then regenerate the generated documentation using go generate
./docs/...; do not edit generated artifacts manually.
In `@pkg/apis/cluster/v1alpha1/types.go`:
- Line 94: The DistributionConfig description in the DistributionConfig field is
incomplete and appears exhaustive; either add KWOK, GKE, and AKS locations or
label the existing paths as examples. Then regenerate the schema and
documentation artifacts using the project’s generation workflow rather than
editing generated files manually.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 5e9c1274-2aa9-4380-b505-e75cfb5ab12d
⛔ Files ignored due to path filters (1)
web/ui/src/generated/ksail-config.tsis excluded by!**/generated/**
📒 Files selected for processing (9)
docs/src/content/docs/configuration/declarative-configuration.mdxpkg/apis/cluster/v1alpha1/autoscaler.gopkg/apis/cluster/v1alpha1/jsonschematag_test.gopkg/apis/cluster/v1alpha1/oidc.gopkg/apis/cluster/v1alpha1/options.gopkg/apis/cluster/v1alpha1/sops.gopkg/apis/cluster/v1alpha1/types.gopkg/svc/chat/docs_generated.goschemas/ksail-config.schema.json
📜 Review details
⏰ Context from checks skipped due to timeout. (16)
- GitHub Check: CodeRabbit
- GitHub Check: 🔍 Dead Code Analysis
- GitHub Check: 🧪 Test
- GitHub Check: 📦 Tidy
- GitHub Check: 📊 Code Coverage
- GitHub Check: 🧹 Lint - mega-linter
- GitHub Check: 🧹 Lint - golangci-lint
- GitHub Check: 🏗️ Build
- GitHub Check: 🛡️ Vulnerability Scan
- GitHub Check: 🏗️ Build KSail Binary
- GitHub Check: ⛵ Operator Chart E2E
- GitHub Check: 🏠 Home Isolation Guard
- GitHub Check: 🏗️ Build (Linux)
- GitHub Check: 🏗️ Build + cask (macOS)
- GitHub Check: Analyze (go)
- GitHub Check: Analyze (go)
🧰 Additional context used
📓 Path-based instructions (3)
pkg/svc/chat/docs_generated.go
📄 CodeRabbit inference engine (AGENTS.md)
Do not hand-edit
pkg/svc/chat/docs_generated.go; regenerate it withgo generate ./pkg/svc/chat/...after the docs generation step.
Files:
pkg/svc/chat/docs_generated.go
docs/src/content/docs/{cli-flags/**,configuration/declarative-configuration.mdx}
📄 CodeRabbit inference engine (AGENTS.md)
Do not hand-edit the generated CLI reference docs under
docs/src/content/docs/cli-flags/ordocs/src/content/docs/configuration/declarative-configuration.mdx; regenerate them withgo generate ./docs/....
Files:
docs/src/content/docs/configuration/declarative-configuration.mdx
schemas/ksail-config.schema.json
📄 CodeRabbit inference engine (AGENTS.md)
Do not hand-edit
schemas/ksail-config.schema.json; regenerate it via the schema generator (go generate ./schemas/...).
Files:
schemas/ksail-config.schema.json
🧠 Learnings (2)
📚 Learning: 2026-07-10T20:56:28.132Z
Learnt from: devantler
Repo: devantler-tech/ksail PR: 6031
File: pkg/svc/chat/docs_generated.go:7-7
Timestamp: 2026-07-10T20:56:28.132Z
Learning: When reviewing code in `pkg/svc/chat/**` that uses `jsonschema:"description=..."` struct tags feeding the `go generate ./pkg/svc/chat/...` docs generator, avoid unescaped/raw commas inside the `description=` value. Struct tag options are comma-separated, so a raw comma will truncate the generated Markdown/MDX table cell text (often right before a `|`). The generator’s `mdxTableCell` correctly escapes literal `|` characters, so truncated output is usually from commas—not pipes. Fix by rewording the description to remove commas (or escaping them according to the tag parser’s rules), then regenerate the docs and ensure the related source-scan regression test passes.
Applied to files:
pkg/svc/chat/docs_generated.go
📚 Learning: 2026-06-30T02:35:51.945Z
Learnt from: devantler
Repo: devantler-tech/ksail PR: 5561
File: pkg/apis/cluster/v1alpha1/options.go:95-108
Timestamp: 2026-06-30T02:35:51.945Z
Learning: For kubebuilder API fields typed as `metav1.Duration`, avoid using `+kubebuilder:validation:Pattern`. `controller-gen` rejects `Pattern` markers on `metav1.Duration` (even though the field may render as a string). Use a CEL `+kubebuilder:validation:XValidation` rule instead to enforce validation constraints, e.g. on duration format/range, so code generation and schema validation both succeed.
Applied to files:
pkg/apis/cluster/v1alpha1/jsonschematag_test.gopkg/apis/cluster/v1alpha1/sops.gopkg/apis/cluster/v1alpha1/oidc.gopkg/apis/cluster/v1alpha1/autoscaler.gopkg/apis/cluster/v1alpha1/options.gopkg/apis/cluster/v1alpha1/types.go
🪛 LanguageTool
docs/src/content/docs/configuration/declarative-configuration.mdx
[style] ~416-~416: To form a complete sentence, be sure to include a subject.
Context: ...he per-pool max values and serverLimit. Should be <= serverLimit | | expander | [...
(MISSING_IT_THERE)
🔇 Additional comments (10)
pkg/svc/chat/docs_generated.go (1)
7-7: LGTM!pkg/apis/cluster/v1alpha1/autoscaler.go (1)
28-28: LGTM!Also applies to: 37-40, 53-53, 99-105
pkg/apis/cluster/v1alpha1/oidc.go (1)
15-29: LGTM!pkg/apis/cluster/v1alpha1/options.go (1)
39-46: LGTM!Also applies to: 86-86, 175-175, 210-216
pkg/apis/cluster/v1alpha1/sops.go (1)
18-18: LGTM!pkg/apis/cluster/v1alpha1/types.go (1)
54-54: LGTM!Also applies to: 135-135, 148-161, 179-184, 290-290
pkg/apis/cluster/v1alpha1/jsonschematag_test.go (1)
1-143: LGTM!schemas/ksail-config.schema.json (2)
362-369: 🗄️ Data Integrity & IntegrationVerify that documented YAML boolean aliases are represented by the schema.
Both
enabledfields are emitted as string enums, while their descriptions promise that YAMLtrue/falsealiases are accepted. Schema-aware editors or validators may reject those values before KSail can apply the loader alias. If aliases are valid schema-validated input, emit a string/booleanoneOf; otherwise clarify that the aliases are outside schema validation.As per coding guidelines, do not hand-edit
schemas/ksail-config.schema.json; regenerate it withgo generate ./schemas/....Also applies to: 441-447
Source: Coding guidelines
552-552: LGTM!docs/src/content/docs/configuration/declarative-configuration.mdx (1)
205-205: LGTM!Also applies to: 414-415
github-project-automation
Bot
moved this from 🫴 Ready
to 🏃🏻♂️ In Progress
in 🌊 Project Board
The comma restoration resurfaced two prose gaps CodeRabbit flagged: the distributionConfig location list read as exhaustive while KWOK/GKE/AKS configs are also supported (now illustrative with the full example set), and maxNodesTotal ended in a sentence fragment. Artifacts regenerated. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Replace timing-based setup cancellation with a context-driven readiness wait, cover SIGINT and SIGTERM through one table, and remove duplicate schema changes now owned by #6036.
@coderabbitai full review Disclosure: This retrigger command was authored by the Daily AI Engineer after validating and pushing commit e6604f1. |
|
✅ Action performedFull review finished. You're currently rate limited under our Fair Usage Limits Policy. Your recent PR review activity is in the 95th percentile or higher among CodeRabbit users, so adaptive limits apply. Your next review will be available in 14 minutes. |
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
schemas/ksail-config.schema.json (1)
526-526: 🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick winDescribe the effective
serverLimitsemantics.Line 826 says
serverLimit: 0uses the default limit of 10, while this description saysmaxNodesTotalshould be<= serverLimit. With the default value, that wording incorrectly implies every positive cap is invalid. Clarify that the comparison uses the effective limit and thatmaxNodesTotal: 0disables the global cap.As per coding guidelines, update the canonical source and regenerate this schema instead of hand-editing the generated file.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@schemas/ksail-config.schema.json` at line 526, Update the canonical configuration source describing maxNodesTotal to state that comparisons use the effective serverLimit (including its default of 10 when configured as 0), while maxNodesTotal: 0 disables the global cap; then regenerate schemas/ksail-config.schema.json rather than editing the generated schema directly.Source: Coding guidelines
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@pkg/apis/cluster/v1alpha1/jsonschematag_test.go`:
- Around line 104-155: Add a field-specific assertion to
TestDistributionConfigDescriptionUsesCanonicalDefaultPaths for
charts/ksail-operator/crds/ksail.io_clusters.yaml, locating the ClusterSpec
distributionConfig schema description and normalizing YAML line wrapping before
comparing it with distributionConfigDescription. Ensure the test fails when the
generated CRD retains stale path text, while avoiding broad file-wide substring
matching.
In `@pkg/apis/cluster/v1alpha1/types.go`:
- Around line 90-94: The DistributionConfig documentation incorrectly says an
empty value uses the distribution’s default “file name”; update the source
comment and jsonschema_description to say “default configuration path.” Then
regenerate all affected schema and documentation artifacts using the project’s
standard generation workflow, without editing generated files manually.
---
Outside diff comments:
In `@schemas/ksail-config.schema.json`:
- Line 526: Update the canonical configuration source describing maxNodesTotal
to state that comparisons use the effective serverLimit (including its default
of 10 when configured as 0), while maxNodesTotal: 0 disables the global cap;
then regenerate schemas/ksail-config.schema.json rather than editing the
generated schema directly.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 7504135d-4cec-4578-9036-5afc27a078be
⛔ Files ignored due to path filters (1)
web/ui/src/generated/ksail-config.tsis excluded by!**/generated/**
📒 Files selected for processing (6)
charts/ksail-operator/crds/ksail.io_clusters.yamldocs/src/content/docs/configuration/declarative-configuration.mdxpkg/apis/cluster/v1alpha1/jsonschematag_test.gopkg/apis/cluster/v1alpha1/types.gopkg/svc/chat/docs_generated.goschemas/ksail-config.schema.json
📜 Review details
🧰 Additional context used
📓 Path-based instructions (3)
pkg/svc/chat/docs_generated.go
📄 CodeRabbit inference engine (AGENTS.md)
Do not hand-edit
pkg/svc/chat/docs_generated.go; regenerate it withgo generate ./pkg/svc/chat/...after the docs generation step.
Files:
pkg/svc/chat/docs_generated.go
schemas/ksail-config.schema.json
📄 CodeRabbit inference engine (AGENTS.md)
Do not hand-edit
schemas/ksail-config.schema.json; regenerate it via the schema generator (go generate ./schemas/...).
Files:
schemas/ksail-config.schema.json
docs/src/content/docs/{cli-flags/**,configuration/declarative-configuration.mdx}
📄 CodeRabbit inference engine (AGENTS.md)
Do not hand-edit the generated CLI reference docs under
docs/src/content/docs/cli-flags/ordocs/src/content/docs/configuration/declarative-configuration.mdx; regenerate them withgo generate ./docs/....
Files:
docs/src/content/docs/configuration/declarative-configuration.mdx
🧠 Learnings (3)
📚 Learning: 2026-07-10T20:56:28.132Z
Learnt from: devantler
Repo: devantler-tech/ksail PR: 6031
File: pkg/svc/chat/docs_generated.go:7-7
Timestamp: 2026-07-10T20:56:28.132Z
Learning: When reviewing code in `pkg/svc/chat/**` that uses `jsonschema:"description=..."` struct tags feeding the `go generate ./pkg/svc/chat/...` docs generator, avoid unescaped/raw commas inside the `description=` value. Struct tag options are comma-separated, so a raw comma will truncate the generated Markdown/MDX table cell text (often right before a `|`). The generator’s `mdxTableCell` correctly escapes literal `|` characters, so truncated output is usually from commas—not pipes. Fix by rewording the description to remove commas (or escaping them according to the tag parser’s rules), then regenerate the docs and ensure the related source-scan regression test passes.
Applied to files:
pkg/svc/chat/docs_generated.go
📚 Learning: 2026-07-10T22:57:33.181Z
Learnt from: devantler
Repo: devantler-tech/ksail PR: 6036
File: docs/src/content/docs/configuration/declarative-configuration.mdx:417-417
Timestamp: 2026-07-10T22:57:33.181Z
Learning: When reviewing declarative-configuration docs (MDX) in ksail, don’t flag as a defect a known generator limitation where `oneOf` unions of scalar-or-array enums are rendered as array-only (e.g., `[]enum` instead of `enum | []enum`) for types like `spec.cluster.autoscaler.node.expander` (tracked in issue `#6037`). If the PR only moves/rewords/edits the JSON schema description content (without changing the docs generator or schema rendering logic), treat this output shape as expected and avoid raising it as a regression.
Applied to files:
docs/src/content/docs/configuration/declarative-configuration.mdx
📚 Learning: 2026-06-30T02:35:51.945Z
Learnt from: devantler
Repo: devantler-tech/ksail PR: 5561
File: pkg/apis/cluster/v1alpha1/options.go:95-108
Timestamp: 2026-06-30T02:35:51.945Z
Learning: For kubebuilder API fields typed as `metav1.Duration`, avoid using `+kubebuilder:validation:Pattern`. `controller-gen` rejects `Pattern` markers on `metav1.Duration` (even though the field may render as a string). Use a CEL `+kubebuilder:validation:XValidation` rule instead to enforce validation constraints, e.g. on duration format/range, so code generation and schema validation both succeed.
Applied to files:
pkg/apis/cluster/v1alpha1/jsonschematag_test.gopkg/apis/cluster/v1alpha1/types.go
🔇 Additional comments (6)
pkg/svc/chat/docs_generated.go (1)
7-7: LGTM!pkg/apis/cluster/v1alpha1/types.go (1)
54-54: LGTM!Also applies to: 135-135, 148-161, 179-184, 290-290
pkg/apis/cluster/v1alpha1/jsonschematag_test.go (1)
13-19: LGTM!schemas/ksail-config.schema.json (1)
217-217: LGTM!Also applies to: 369-369, 447-447, 552-552
charts/ksail-operator/crds/ksail.io_clusters.yaml (1)
306-307: LGTM!docs/src/content/docs/configuration/declarative-configuration.mdx (1)
205-205: LGTM!Also applies to: 414-417
…, CRD guard - 'default file name' -> 'default configuration path' (talos/ and kwok/ are directories), regenerated everywhere including the operator CRD - maxNodesTotal description now states comparisons use the EFFECTIVE serverLimit (0 = default 10) and that 0 disables the global cap - new TestGeneratedCRDCarriesCurrentDescriptions pins the chart CRD to the doc comments it renders from (it was stale on main)
…nto claude/jsonschema-desc-commas # Conflicts: # charts/ksail-operator/crds/ksail.io_clusters.yaml # pkg/apis/cluster/v1alpha1/jsonschematag_test.go # pkg/apis/cluster/v1alpha1/types.go # pkg/svc/chat/docs_generated.go
![ksail-bot[bot]](https://avatars.githubusercontent.com/in/2880167?s=60&v=4){kind=small}
Outside-diff finding (schemas/ksail-config.schema.json L526, maxNodesTotal vs serverLimit): fixed in 7099f7d. The canonical @coderabbitai review |
|
✅ Action performedReview finished.
|
1 participant
Why
Commas inside inline schema descriptions were silently truncating what users see in IDE hovers, the docs configuration reference, and the web UI types — four fields shipped with cut-off text, and any future comma would do the same.
What
Moves every inline description to the dedicated comma-safe tag (30 fields), restoring the four truncated descriptions verbatim in all regenerated artifacts, and adds a guard test so the whole class cannot recur.
Fixes #6035