SDL parity tests #169
SDL parity tests #169
Conversation
WalkthroughAdds SDL parity tooling: schemas, many v2.0/v2.1 and invalid fixtures, Go fixture generator, Go/TS parity tests, schema validator integration in Go, deterministic ordering fixes, Make/CI targets, and supporting package dependency updates. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor Dev as Developer
participant CI as GitHub Actions
participant Make as Makefile
participant Gen as Go Fixture Generator
participant GoTest as Go parity tests
participant TSTest as TS parity tests
participant Schema as Schema Validator
Dev->>CI: push PR / trigger workflow
CI->>Make: run tests
Make->>Gen: generate-sdl-fixtures
Gen->>Gen: read `input.yaml` fixtures
Gen->>Gen: call sdl.Manifest() / sdl.DeploymentGroups()
Gen->>Gen: write `manifest.json` & `groups.json`
CI->>GoTest: run Go parity tests
GoTest->>Schema: validate input against sdl-input.schema.yaml
GoTest->>GoTest: parse input -> Manifest & Groups
GoTest->>Schema: validate outputs against manifest/groups schemas
GoTest->>GoTest: compare generated vs fixture JSON
CI->>TSTest: run TypeScript parity tests
TSTest->>Schema: validate input against sdl-input.schema.yaml (AJV)
TSTest->>TSTest: SDL.fromString -> v3Manifest(go) + v3Groups(go)
TSTest->>Schema: validate outputs against manifest/groups schemas
TSTest->>TSTest: compare generated vs fixture JSON
CI->>Dev: report results
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes
Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (2)
⏰ 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). (5)
🔇 Additional comments (4)
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 |
| if len(svc.Params.Storage) > 0 { | ||
| params.Storage = make([]manifest.StorageParams, 0, len(svc.Params.Storage)) | ||
| for volName, volParams := range svc.Params.Storage { | ||
| storageNames := make([]string, 0, len(svc.Params.Storage)) |
There was a problem hiding this comment.
Added sorting to ensure deterministic output from Go map iteration
| if len(svc.Params.Storage) > 0 { | ||
| params.Storage = make([]manifest.StorageParams, 0, len(svc.Params.Storage)) | ||
| for volName, volParams := range svc.Params.Storage { | ||
| storageNames := make([]string, 0, len(svc.Params.Storage)) |
There was a problem hiding this comment.
Added sorting to ensure deterministic output from Go map iteration
| { | ||
| "mount": "/var/lib/mysql", | ||
| "name": "wordpress-db", | ||
| "mount": "/dev/shm", |
There was a problem hiding this comment.
this snapshot was changed because I added sorting
vertex451
force-pushed
the
artem/sdl-parity-tests
branch
from
c464548 to
c669081
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
go/sdl/go.mod (1)
7-15: gojsonschema dependency is used in production JWT schema validationThe
gojsonschemadependency is imported ingo/util/jwt/schema.gofor production JWT schema validation (initialized at package level), in addition to its use in SDL parity tests. The dependency is appropriate for these use cases, though be aware it initializes at package load time and uses reflection, which incurs startup overhead.ts/src/sdl/SDL/SDL.ts (1)
834-846: Essential change: Deterministic storage ordering for parity testing.Sorting storage keys alphabetically before mapping ensures consistent, reproducible manifest output. This is critical for cross-language parity validation.
♻️ Duplicate comments (1)
go/sdl/groupBuilder_v2.go (1)
100-121: Sorted iteration over storage volumes for deterministic manifestsUsing
storageNames+sort.Stringsbefore appendingStorageParamseliminates Go map iteration nondeterminism and keeps v2 output stable across runs and in parity tests. Implementation mirrors the v2.1 path and looks correct.
🧹 Nitpick comments (11)
go/sdl/groupBuilder_v2_1.go (1)
100-131: Deterministic storage params ordering is correctSorting
svc.Params.Storagekeys before buildingparams.Storageremoves map‑iteration nondeterminism and aligns with parity/determinism needs. Logic is sound and nil‑safe; any future refactor could share this helper with the v2 builder, but it’s not required now.make/test.mk (1)
51-63: Parity test workflow is solid; align TS path with existingTS_ROOTThe new
generate-sdl-fixturesandtest-sdl-paritytargets wire the Go and TS parity tests together cleanly, and the sequencing (fixtures → Go tests → TS tests) makes sense.For consistency with
test-ts/test-coverage-ts, consider using$(TS_ROOT)instead of a hard-codedtspath:- @cd ts && npm test -- --testPathPattern=parity.spec.ts + @cd $(TS_ROOT) && npm test -- --testPathPattern=parity.spec.tsThis avoids future drift if the TS root directory is ever renamed.
go/sdl/cmd/generate-sdl-fixtures/main.go (1)
13-13: Consider making the fixtures root path configurable.The hard-coded relative path
"../../../testdata/sdl"assumes the command is executed from a specific directory (likelygo/sdl/cmd/generate-sdl-fixtures). If the tool is run from a different location or installed elsewhere, it will fail.Consider accepting the fixtures root as a command-line argument with a default value:
+import ( + "flag" + // ... existing imports +) + func main() { - fixturesRoot := "../../../testdata/sdl" + fixturesRoot := flag.String("fixtures", "../../../testdata/sdl", "Path to SDL fixtures root directory") + flag.Parse() + versions := []string{"v2.0", "v2.1"} for _, version := range versions { - versionDir := filepath.Join(fixturesRoot, version) + versionDir := filepath.Join(*fixturesRoot, version)testdata/sdl/v2.0/persistent-storage/groups.json (1)
1-87: Add missing trailing newline.The file is missing a trailing newline at EOF. While some build systems are forgiving, most linters and Git conventions expect JSON files to end with a newline character for POSIX compliance.
Add a newline after the closing bracket at line 87.
] +testdata/sdl/v2.0/simple/groups.json (1)
1-67: Add missing trailing newline.Per the file structure, the JSON is valid but ends without a trailing newline. Add a newline after the closing bracket for POSIX compliance.
] +testdata/sdl/v2.0/storage-classes/groups.json (1)
1-87: Add missing trailing newline.The file is missing a trailing newline at EOF, violating POSIX conventions and typical linter expectations.
] +specs/sdl/sdl-input.schema.yaml (1)
10-24: Consider usingintegertype for counters instead ofnumberFields like
deployment.*.*.count(and similar count-like fields elsewhere) are conceptually integers. Using"type": "integer"instead of"type": "number"would prevent fractional values from passing schema validation while keeping current fixtures valid.Also applies to: 41-65
specs/sdl/groups.schema.yaml (1)
45-65: Tighten groups schema forcountandprice.denomTo better reflect the data model and avoid subtle mistakes:
resources[*].countshould be an integer (non-negative) rather than a genericnumber.pricecurrently requiresamountbut notdenom; in practice both are always present, so makingdenomrequired too would strengthen validation without affecting existing fixtures.go/sdl/parity_test.go (1)
17-46: Optional: reuse compiled JSON Schemas across fixtures
validateAgainstSchemareads and compiles the schema file for every manifest/groups validation. With many fixtures this adds unnecessary overhead. Consider caching compiled schemas keyed byschemaPath(e.g., package-levelmap[string]*gojsonschema.Schemaprotected bysync.Mutex/sync.Once) so each schema file is loaded and compiled only once per test run.Also applies to: 94-118
ts/src/sdl/SDL/parity.spec.ts (1)
81-101: Optional: cache compiled schemas to avoid recompiling on every fixture
validateAgainstSchemareads and compiles the YAML schema on every call. For many fixtures this adds overhead in test runs. You could cache the compiled Ajv validators byschemaPath(e.g., a simpleMap<string, ValidateFunction>) so each schema file is parsed/compiled only once per test run. This follows Ajv's recommended pattern of compiling once and reusing the validate function rather than recompiling on each invocation.specs/sdl/manifest.schema.yaml (1)
1-3: Add $schema and $id metadata for schema documentation and tooling.This is a new public schema file. It lacks standard JSON Schema metadata fields that aid documentation, versioning, and integration with schema validators.
Add
$schema(to declare the JSON Schema version) and$id(for schema identification and URL resolution) at the root level:+$schema: 'https://json-schema.org/draft/2020-12/schema' +$id: 'https://akash.network/schemas/sdl/manifest.schema.json' +title: Akash SDL Manifest Schema +description: Validates the structure of a compiled SDL manifest, including services, resources, and deployment configurations. items: additionalProperties: false properties:Also applies to: 328-328
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (3)
go/sdl/go.sumis excluded by!**/*.sumts/package-lock.jsonis excluded by!**/package-lock.jsonts/src/sdl/SDL/__snapshots__/SDL.spec.ts.snapis excluded by!**/*.snap
📒 Files selected for processing (50)
.github/workflows/tests.yaml(1 hunks).gitignore(1 hunks)go/sdl/cmd/generate-sdl-fixtures/main.go(1 hunks)go/sdl/go.mod(2 hunks)go/sdl/groupBuilder_v2.go(1 hunks)go/sdl/groupBuilder_v2_1.go(1 hunks)go/sdl/parity_test.go(1 hunks)make/test.mk(1 hunks)specs/sdl/groups.schema.yaml(1 hunks)specs/sdl/manifest.schema.yaml(1 hunks)specs/sdl/sdl-input.schema.yaml(1 hunks)specs/sdl/validation-limitations.md(1 hunks)testdata/sdl/invalid/credentials-missing-host.yaml(1 hunks)testdata/sdl/invalid/endpoint-not-used.yaml(1 hunks)testdata/sdl/invalid/invalid-port.yaml(1 hunks)testdata/sdl/invalid/missing-deployment.yaml(1 hunks)testdata/sdl/invalid/missing-image.yaml(1 hunks)testdata/sdl/invalid/missing-version.yaml(1 hunks)testdata/sdl/invalid/negative-cpu.yaml(1 hunks)testdata/sdl/invalid/persistent-without-mount.yaml(1 hunks)testdata/sdl/v2.0/gpu-basic/groups.json(1 hunks)testdata/sdl/v2.0/gpu-basic/input.yaml(1 hunks)testdata/sdl/v2.0/gpu-basic/manifest.json(1 hunks)testdata/sdl/v2.0/http-options/groups.json(1 hunks)testdata/sdl/v2.0/http-options/input.yaml(1 hunks)testdata/sdl/v2.0/http-options/manifest.json(1 hunks)testdata/sdl/v2.0/ip-endpoint/groups.json(1 hunks)testdata/sdl/v2.0/ip-endpoint/input.yaml(1 hunks)testdata/sdl/v2.0/ip-endpoint/manifest.json(1 hunks)testdata/sdl/v2.0/multiple-services/groups.json(1 hunks)testdata/sdl/v2.0/multiple-services/input.yaml(1 hunks)testdata/sdl/v2.0/multiple-services/manifest.json(1 hunks)testdata/sdl/v2.0/persistent-storage/groups.json(1 hunks)testdata/sdl/v2.0/persistent-storage/input.yaml(1 hunks)testdata/sdl/v2.0/persistent-storage/manifest.json(1 hunks)testdata/sdl/v2.0/simple/groups.json(1 hunks)testdata/sdl/v2.0/simple/input.yaml(1 hunks)testdata/sdl/v2.0/simple/manifest.json(1 hunks)testdata/sdl/v2.0/storage-classes/groups.json(1 hunks)testdata/sdl/v2.0/storage-classes/input.yaml(1 hunks)testdata/sdl/v2.0/storage-classes/manifest.json(1 hunks)testdata/sdl/v2.1/credentials/groups.json(1 hunks)testdata/sdl/v2.1/credentials/input.yaml(1 hunks)testdata/sdl/v2.1/credentials/manifest.json(1 hunks)testdata/sdl/v2.1/ip-endpoint/groups.json(1 hunks)testdata/sdl/v2.1/ip-endpoint/input.yaml(1 hunks)testdata/sdl/v2.1/ip-endpoint/manifest.json(1 hunks)ts/package.json(1 hunks)ts/src/sdl/SDL/SDL.ts(9 hunks)ts/src/sdl/SDL/parity.spec.ts(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (4)
ts/src/sdl/SDL/parity.spec.ts (1)
ts/src/sdl/SDL/SDL.ts (3)
validate(128-138)validate(246-261)SDL(88-1315)
go/sdl/cmd/generate-sdl-fixtures/main.go (1)
go/sdl/sdl.go (1)
ReadFile(83-89)
go/sdl/parity_test.go (2)
go/sdl/sdl.go (1)
ReadFile(83-89)go/testutil/deployment.go (1)
DeploymentGroups(54-62)
ts/src/sdl/SDL/SDL.ts (2)
ts/src/sdl/index.ts (1)
SdlValidationError(28-28)ts/src/sdl/types.ts (1)
v3Sdl(100-105)
🪛 GitHub Actions: lint
go/sdl/cmd/generate-sdl-fixtures/main.go
[error] 87-87: gofmt formatting issue detected. File is not properly formatted. Run 'gofmt -w' or 'go fmt ./...' to fix.
⏰ 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). (1)
- GitHub Check: coverage
🔇 Additional comments (42)
.gitignore (1)
27-34: Well-organized additions supporting the testing infrastructure.The new entries appropriately capture test/build artifacts and coverage outputs introduced by the SDL parity testing changes. The naming conventions align with existing patterns, and the logical grouping with comments enhances clarity.
testdata/sdl/invalid/missing-image.yaml (1)
1-30: Invalid SDL fixture for missing image looks correctSDL structure is consistent with other v2.0 fixtures, and omitting
services.web.imageshould exercise the intended invalid case.testdata/sdl/invalid/invalid-port.yaml (1)
1-32: Invalid‑port SDL fixture is well‑formed for the negative testThe SDL is structurally valid but uses
port: 99999, which is out of the allowed port range, making it a good fixture to assert that validation rejects out‑of‑range ports.testdata/sdl/v2.0/ip-endpoint/manifest.json (1)
1-103: ip‑endpoint manifest fixture appears structurally consistentThe manifest structure (group → service → resources/endpoints → expose with
ipandendpointSequenceNumber) looks consistent with the v2.0/ip‑endpoint scenario and suitable for parity tests. Assuming this JSON was generated by the fixture tool, it should accurately represent the expected manifest; if it was edited manually, it’s worth re‑generating once to ensure it matches generator output and the companiongroups.json.testdata/sdl/invalid/negative-cpu.yaml (1)
1-32: Negative‑CPU SDL fixture correctly targets invalid resource valuesUsing
cpu.units: -100min an otherwise valid deployment is an appropriate way to exercise validation for disallowing negative CPU values.ts/package.json (1)
69-81: AJV is correctly placed as a devDependencyAJV (
^8.12.0) is used only in test files (ts/src/sdl/SDL/parity.spec.ts), confirming it should remain indevDependenciesand not be added todependencies. No runtime code imports or depends on AJV.testdata/sdl/invalid/credentials-missing-host.yaml (1)
1-35: LGTM! Invalid fixture correctly tests missing host credential validation.This fixture appropriately tests the credential validation pathway by intentionally omitting the required
hostfield while providingusernameandpassword. The validation logic ints/src/sdl/SDL/SDL.ts(lines 301-310) will correctly reject this SDL.testdata/sdl/v2.0/http-options/manifest.json (1)
1-73: LGTM! HTTP options manifest fixture is well-structured.The manifest correctly defines HTTP options with appropriate retry policies and timeout configurations for parity testing.
testdata/sdl/v2.0/storage-classes/manifest.json (1)
1-117: LGTM! Storage classes manifest correctly demonstrates multiple storage volumes.The fixture properly defines storage with varying attributes (persistent flags, storage classes) and the storage params section follows the alphabetically sorted order (cache, data, logs) as enforced by the updated SDL.ts logic.
testdata/sdl/invalid/endpoint-not-used.yaml (1)
1-35: LGTM! Invalid fixture correctly tests unused endpoint detection.This fixture appropriately validates that endpoints defined but never referenced in service exposures are detected as errors. The
validateEndpointsUtilitymethod in SDL.ts will correctly flag this scenario.testdata/sdl/v2.0/gpu-basic/manifest.json (1)
1-76: LGTM! GPU manifest fixture correctly represents GPU configuration.The fixture properly defines GPU resources with NVIDIA RTX 3080 specification, suitable for testing GPU-enabled deployment parity.
ts/src/sdl/SDL/SDL.ts (5)
2-2: LGTM! Named import improves clarity.Using the named
loadimport fromjs-yamlis more explicit and follows better practices than relying on default exports.
114-121: LGTM! Version field validation enhances robustness.Adding an explicit check for the required
versionfield with a clear error message improves SDL validation. The type cast tov3Sdlappropriately follows the validation.
285-299: LGTM! Service image and port validations strengthen SDL integrity.Both validation methods correctly enforce essential constraints:
- Empty image names are rejected
- Port values are constrained to valid range (1-65535)
The error messages are clear and helpful.
848-864: LGTM! Proper null handling and deterministic storage ordering.The method correctly handles
undefined/nullparams by returningnull, and ensures deterministic storage parameter ordering through alphabetical sorting.
899-916: LGTM! Conditional params assignment prevents unnecessary fields.Computing params via
v3ManifestServiceParamsand only assigning when not null prevents adding empty params fields to the manifest, resulting in cleaner output..github/workflows/tests.yaml (1)
57-76: LGTM! SDL parity CI job is properly configured.The new job correctly mirrors the setup steps from the coverage job and executes the SDL parity tests. The setup includes both Node.js and Go environments as required for cross-language parity validation.
testdata/sdl/v2.1/credentials/manifest.json (1)
1-75: LGTM! Credentials manifest correctly includes all required fields.This fixture appropriately tests the valid credentials scenario with all required fields (host, email, username, password) present, complementing the invalid credentials fixtures.
testdata/sdl/v2.0/gpu-basic/groups.json (1)
1-65: GPU basic group fixture looks structurally consistentThe JSON structure (requirements/resources/price) is consistent with other group fixtures, and the numeric values (CPU/memory/storage/GPU/price) look coherent for a minimal GPU case.
testdata/sdl/v2.0/multiple-services/groups.json (1)
1-130: Multiple-services group fixture is coherent and well-formedRequirements, resource variants, and pricing are internally consistent and align with the structure used in the other group fixtures in this PR.
testdata/sdl/invalid/missing-deployment.yaml (1)
1-27: Invalid “missing deployment” fixture matches its intentYAML is structurally valid, and omitting the
deploymentblock accurately represents the intended invalid case for tests.testdata/sdl/invalid/persistent-without-mount.yaml (1)
1-35: Persistent-without-mount invalid fixture is clearly definedThe compute/storage and deployment sections are well-formed, and the missing mount for a
persistent: truevolume cleanly expresses the invalid scenario the tests will exercise.testdata/sdl/v2.0/http-options/input.yaml (1)
1-50: HTTP-options SDL fixture is well-structured and expressiveThe
http_optionsblock and its parameters (timeouts, retries, next_cases) are clearly defined, and the compute/placement/deployment sections follow the established SDL v2.0 pattern, making this a good parity fixture.testdata/sdl/v2.0/ip-endpoint/input.yaml (1)
1-49: IP-endpoint SDL fixture cleanly exercises named IP exposureThe service/expose blocks correctly reference
ip: myendpoint, and the matchingendpoints.myendpointdefinition (kindip) plus standard compute/placement/deployment make this a solid parity case.testdata/sdl/v2.1/credentials/groups.json (1)
1-59: Credentials group fixture matches existing group schema usageThe requirements, resource definition, and pricing fields align with the other v2.0/v2.1 group fixtures and look appropriate for a credentials-focused parity test.
testdata/sdl/v2.0/simple/input.yaml (1)
1-47: LGTM! Well-structured SDL v2.0 test fixture.The fixture provides good coverage with multiple expose configurations (HTTP and UDP), placement attributes, and signedBy constraints with both anyOf and allOf.
testdata/sdl/v2.0/persistent-storage/manifest.json (1)
1-92: LGTM! Comprehensive manifest fixture for persistent storage.The manifest correctly captures persistent storage configuration with volume attributes (persistent: true, class: beta2) and mount parameters. The structure aligns with the expected manifest schema for v2.0.
testdata/sdl/invalid/missing-version.yaml (1)
1-31: LGTM! Valid negative test case for missing version.This fixture appropriately tests SDL validation by omitting the required
versionfield while maintaining otherwise valid structure. This ensures error handling for missing version information is properly exercised.testdata/sdl/v2.0/gpu-basic/input.yaml (1)
22-27: LGTM! Well-structured GPU resource configuration.The GPU attributes correctly specify the vendor (nvidia) and model (rtx3080) in nested format, which aligns with SDL v2.0 GPU specifications. This provides good test coverage for GPU-enabled deployments.
testdata/sdl/v2.0/http-options/groups.json (1)
1-59: LGTM! Valid groups fixture with proper structure.The groups data correctly defines requirements (signed_by, attributes), resources (compute specs, endpoints), and pricing. The 18-decimal precision for the amount field is standard for cryptocurrency denominations.
testdata/sdl/v2.1/ip-endpoint/input.yaml (1)
67-72: LGTM! Proper v2.1 IP endpoint configuration.The endpoints section correctly defines IP endpoints (endpoint1, endpoint2) which are referenced by the services' expose configurations. This provides good test coverage for the v2.1 IP endpoint feature.
testdata/sdl/v2.0/persistent-storage/input.yaml (1)
13-17: LGTM! Correct persistent storage configuration.The fixture properly links storage parameters (lines 14-17) with the compute profile's storage definition (lines 27-31). The storage volume is correctly marked as persistent with a storage class, and the mount point is properly specified.
Also applies to: 27-31
go/sdl/cmd/generate-sdl-fixtures/main.go (1)
60-60: File permissions0644are appropriate for generated test fixtures.The codebase shows a consistent pattern: test and configuration files use
0644permissions (owner read/write, group/others read-only), while sensitive files like key imports and governance data use0o600. Since the manifest and groups JSON files are test fixtures intended to be committed and read by the build system,0644is the correct choice.specs/sdl/validation-limitations.md (1)
1-12: All file references in the documentation are accurate. The schema filespecs/sdl/sdl-input.schema.yamland parser filesgo/sdl/v2.goandgo/sdl/v2_1.goall exist at the paths specified in the documentation.testdata/sdl/v2.1/ip-endpoint/manifest.json (1)
36-45: Verify if mixed endpoint structures between services are intentional.The manifest defines asymmetric endpoint configurations: the "api" service (lines 36-45) has consistent endpoints with both
kindandsequence_number, while the "web" service (lines 104-120) has mixed endpoints—some with onlysequence_number, others with both fields. Confirm whether this variation represents intentional test coverage for different endpoint types or if it's an inconsistency requiring alignment.Also applies to: 104-120
testdata/sdl/v2.1/credentials/input.yaml (1)
1-43: Schema reference and fixture consistency verified.The relative schema path
../../../../specs/sdl/sdl-input.schema.yamlis correct and the schema file exists. The input.yaml file is consistent with its corresponding groups.json and manifest.json fixtures: the service "private-app" maps to the "akash" group, and all resource specifications (CPU 500m, memory 1Gi, storage 5Gi, pricing 250 uakt) and credentials match across all three files.testdata/sdl/v2.1/ip-endpoint/groups.json (1)
1-114: ip-endpoint groups fixture shape looks consistent with schemaThe group structure (requirements.signed_by/attributes, resources with cpu/memory/storage/gpu/endpoints, count, and price) matches the new
groups.schema.yamlexpectations and provides good coverage of endpoint kinds/sequence numbers. I don’t see structural or obvious value issues here.testdata/sdl/v2.0/storage-classes/input.yaml (1)
1-59: Storage-classes SDL input aligns with the new SDL input schemaService, profiles (compute/placement), and deployment sections — including per-volume storage attributes and the
signedBy/pricing structure — all line up withsdl-input.schema.yaml. This fixture should be a solid basis for exercising storage-class handling.testdata/sdl/v2.0/multiple-services/manifest.json (1)
1-193: multiple-services manifest fixture is internally consistentService definitions (images, env, counts), inter-service expose wiring, resource IDs, and endpoint/
httpOptionsstructures are coherent and match what the v2.0 multiple-services SDL input describes. This looks like a good “golden” manifest for parity tests.testdata/sdl/v2.0/multiple-services/input.yaml (1)
1-93: multiple-services SDL input matches schema and manifest/groups fixturesThe service graph (web → api → db), compute/placement profiles, pricing, and deployment counts are coherent and line up with the generated manifest/groups fixtures. The
$schemaheader also correctly referencessdl-input.schema.yaml.specs/sdl/manifest.schema.yaml (2)
140-157: Clarify intentional structural difference between params.storage and resources.storage.Storage appears in two locations with differing structures:
- params.storage (lines 140-157): Array of objects with
{name, mount, readOnly}- resources.storage (lines 277-313): Array of objects with
{name, size, attributes}These serve different purposes, but the schema does not document this. Confirm this is intentional and document the distinction.
Verify that these storage structures represent different semantic concepts (e.g., params storage = mount points, resources storage = resource specifications) and not a schema inconsistency.
Also applies to: 277-313
67-92: httpOptions is universally required and consistently applied across all protocols in valid manifests.Verification of test fixtures confirms that all nine manifest files (v2.0 and v2.1) include httpOptions for every expose entry, regardless of protocol type (TCP, UDP, HTTP). No counter-examples exist where expose entries lack httpOptions. Non-HTTP protocols like TCP and UDP are fully supported and already accompanied by httpOptions in production fixtures, indicating this is an intentional schema design rather than an oversight. The concern that non-HTTP services would be rejected is unfounded—they are validated and expected to include httpOptions as part of the uniform expose specification.
vertex451
force-pushed
the
artem/sdl-parity-tests
branch
from
c669081 to
37393d8
Compare
vertex451
force-pushed
the
artem/sdl-parity-tests
branch
from
37393d8 to
204e70b
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
ts/src/sdl/SDL/parity.spec.ts (1)
163-169: Bug:finalValueis computed but immediately overwritten, making the clamping logic a no-op.This was flagged in a previous review. Line 167 assigns
finalValue(with the clampedendpointSequenceNumber), but line 169 immediately overwrites it with the originalvalue, negating allfinalValueadjustments.if (key === "endpointSequenceNumber" && typeof value === "number" && value > 1) { finalValue = 1; } normalized[normalizedKey] = finalValue; - - normalized[normalizedKey] = value;
🧹 Nitpick comments (4)
specs/sdl/sdl-input.schema.yaml (2)
211-215: Fix indentation inconsistency foraccept.items.The
itemsandtypeunderaccepthave extra indentation compared to sibling properties.accept: - items: - type: string - type: array + items: + type: string + type: array
287-291: Consider addingservicesandprofilesto required fields.Currently only
versionanddeploymentare required. While semantic validation is deferred to runtime, a valid SDL structurally requiresservicesandprofilessections. Adding them torequiredwould catch malformed SDLs earlier during IDE validation.required: - version - deployment + - services + - profilests/src/sdl/SDL/SDL.ts (1)
290-299: LGTM with minor suggestion.Port validation logic is correct. Consider a clearer error message:
- `Service "${serviceName}" has invalid port value. Port must be 0 < value <= 65535.`, + `Service "${serviceName}" has invalid port value. Port must be between 1 and 65535.`,ts/src/sdl/SDL/parity.spec.ts (1)
404-413: Consider testing invalid fixtures against both beta2 and beta3.Invalid fixtures are only tested with
beta3. If validation differs between versions (e.g., the invalid-port.yaml uses version "2.0"), testing only beta3 may miss version-specific edge cases.entries.forEach((filename) => { ["beta2", "beta3"].forEach((version) => { it(`should reject ${filename} (${version})`, () => { const input = fs.readFileSync( path.join(invalidDir, filename), "utf8", ); expect(() => SDL.fromString(input, version as "beta2" | "beta3")).toThrow(); }); }); });
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (3)
go/sdl/go.sumis excluded by!**/*.sumts/package-lock.jsonis excluded by!**/package-lock.jsonts/src/sdl/SDL/__snapshots__/SDL.spec.ts.snapis excluded by!**/*.snap
📒 Files selected for processing (50)
.github/workflows/tests.yaml(1 hunks).gitignore(1 hunks)go/sdl/cmd/generate-sdl-fixtures/main.go(1 hunks)go/sdl/go.mod(2 hunks)go/sdl/groupBuilder_v2.go(1 hunks)go/sdl/groupBuilder_v2_1.go(1 hunks)go/sdl/parity_test.go(1 hunks)make/test.mk(1 hunks)specs/sdl/groups.schema.yaml(1 hunks)specs/sdl/manifest.schema.yaml(1 hunks)specs/sdl/sdl-input.schema.yaml(1 hunks)specs/sdl/validation-limitations.md(1 hunks)testdata/sdl/invalid/credentials-missing-host.yaml(1 hunks)testdata/sdl/invalid/endpoint-not-used.yaml(1 hunks)testdata/sdl/invalid/invalid-port.yaml(1 hunks)testdata/sdl/invalid/missing-deployment.yaml(1 hunks)testdata/sdl/invalid/missing-image.yaml(1 hunks)testdata/sdl/invalid/missing-version.yaml(1 hunks)testdata/sdl/invalid/negative-cpu.yaml(1 hunks)testdata/sdl/invalid/persistent-without-mount.yaml(1 hunks)testdata/sdl/v2.0/gpu-basic/groups.json(1 hunks)testdata/sdl/v2.0/gpu-basic/input.yaml(1 hunks)testdata/sdl/v2.0/gpu-basic/manifest.json(1 hunks)testdata/sdl/v2.0/http-options/groups.json(1 hunks)testdata/sdl/v2.0/http-options/input.yaml(1 hunks)testdata/sdl/v2.0/http-options/manifest.json(1 hunks)testdata/sdl/v2.0/ip-endpoint/groups.json(1 hunks)testdata/sdl/v2.0/ip-endpoint/input.yaml(1 hunks)testdata/sdl/v2.0/ip-endpoint/manifest.json(1 hunks)testdata/sdl/v2.0/multiple-services/groups.json(1 hunks)testdata/sdl/v2.0/multiple-services/input.yaml(1 hunks)testdata/sdl/v2.0/multiple-services/manifest.json(1 hunks)testdata/sdl/v2.0/persistent-storage/groups.json(1 hunks)testdata/sdl/v2.0/persistent-storage/input.yaml(1 hunks)testdata/sdl/v2.0/persistent-storage/manifest.json(1 hunks)testdata/sdl/v2.0/simple/groups.json(1 hunks)testdata/sdl/v2.0/simple/input.yaml(1 hunks)testdata/sdl/v2.0/simple/manifest.json(1 hunks)testdata/sdl/v2.0/storage-classes/groups.json(1 hunks)testdata/sdl/v2.0/storage-classes/input.yaml(1 hunks)testdata/sdl/v2.0/storage-classes/manifest.json(1 hunks)testdata/sdl/v2.1/credentials/groups.json(1 hunks)testdata/sdl/v2.1/credentials/input.yaml(1 hunks)testdata/sdl/v2.1/credentials/manifest.json(1 hunks)testdata/sdl/v2.1/ip-endpoint/groups.json(1 hunks)testdata/sdl/v2.1/ip-endpoint/input.yaml(1 hunks)testdata/sdl/v2.1/ip-endpoint/manifest.json(1 hunks)ts/package.json(1 hunks)ts/src/sdl/SDL/SDL.ts(9 hunks)ts/src/sdl/SDL/parity.spec.ts(1 hunks)
✅ Files skipped from review due to trivial changes (2)
- specs/sdl/validation-limitations.md
- testdata/sdl/invalid/negative-cpu.yaml
🚧 Files skipped from review as they are similar to previous changes (37)
- go/sdl/groupBuilder_v2.go
- .github/workflows/tests.yaml
- testdata/sdl/invalid/endpoint-not-used.yaml
- testdata/sdl/invalid/missing-version.yaml
- testdata/sdl/v2.0/persistent-storage/manifest.json
- go/sdl/parity_test.go
- make/test.mk
- testdata/sdl/v2.0/storage-classes/groups.json
- go/sdl/go.mod
- testdata/sdl/v2.0/multiple-services/groups.json
- testdata/sdl/v2.1/ip-endpoint/manifest.json
- testdata/sdl/v2.0/ip-endpoint/manifest.json
- testdata/sdl/v2.0/multiple-services/manifest.json
- testdata/sdl/v2.0/simple/groups.json
- testdata/sdl/v2.0/gpu-basic/groups.json
- testdata/sdl/invalid/missing-deployment.yaml
- testdata/sdl/v2.0/http-options/input.yaml
- testdata/sdl/v2.0/simple/manifest.json
- testdata/sdl/v2.0/http-options/groups.json
- testdata/sdl/invalid/missing-image.yaml
- testdata/sdl/v2.0/ip-endpoint/groups.json
- ts/package.json
- go/sdl/cmd/generate-sdl-fixtures/main.go
- testdata/sdl/v2.0/gpu-basic/manifest.json
- testdata/sdl/v2.0/storage-classes/manifest.json
- specs/sdl/manifest.schema.yaml
- testdata/sdl/v2.1/credentials/groups.json
- testdata/sdl/v2.0/simple/input.yaml
- testdata/sdl/invalid/credentials-missing-host.yaml
- testdata/sdl/v2.1/ip-endpoint/input.yaml
- testdata/sdl/v2.0/gpu-basic/input.yaml
- testdata/sdl/v2.1/ip-endpoint/groups.json
- testdata/sdl/invalid/persistent-without-mount.yaml
- testdata/sdl/v2.1/credentials/manifest.json
- specs/sdl/groups.schema.yaml
- .gitignore
- testdata/sdl/v2.0/http-options/manifest.json
🧰 Additional context used
🧬 Code graph analysis (1)
ts/src/sdl/SDL/SDL.ts (2)
ts/src/sdl/index.ts (1)
SdlValidationError(28-28)ts/src/sdl/types.ts (1)
v3Sdl(100-105)
⏰ 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). (3)
- GitHub Check: coverage
- GitHub Check: test
- GitHub Check: go
🔇 Additional comments (14)
testdata/sdl/v2.0/persistent-storage/input.yaml (1)
3-48: SDL configuration is well-formed and consistent.The YAML SDL v2.0 configuration correctly defines:
- Persistent storage setup with Postgres database service
- Compute profile with appropriate resource specifications (500m CPU, 1Gi RAM, 50Gi storage)
- Placement constraints (region us-west, signed_by policy)
- Pricing and deployment mappings
Cross-referenced against the companion
groups.jsonfixture, resource specifications are consistent.testdata/sdl/v2.0/persistent-storage/groups.json (1)
1-70: JSON fixture data is accurate and consistent with input.yaml.Byte conversions are correct:
- Memory: 1,073,741,824 bytes = 1 GiB ✓
- Storage: 53,687,091,200 bytes = 50 GiB ✓
All resource specifications (CPU 500 units, storage class/attributes, pricing, count) align with the corresponding input.yaml file. The data structure appears well-suited for parity test validation.
testdata/sdl/v2.0/storage-classes/input.yaml (2)
1-1: The schema reference path is correct and resolves properly. The filespecs/sdl/sdl-input.schema.yamlexists at the expected location, and the relative path../../../../specs/sdl/sdl-input.schema.yamlcorrectly references the SDL input schema from the file's location attestdata/sdl/v2.0/storage-classes/input.yaml.
2-58: LGTM! Comprehensive storage-classes fixture.The SDL v2.0 manifest provides a well-structured test fixture that exercises the storage-classes feature with parametric storage definitions, mixed storage attributes (persistent flags and class names), and proper deployment configuration. The mixed attribute patterns (cache/logs minimal vs. data with both persistent and class) are intentional for testing edge cases. The fixture validates successfully against the sdl-input.schema.yaml schema.
go/sdl/groupBuilder_v2_1.go (1)
115-121: LGTM! Deterministic ordering ensures parity testing reliability.The implementation correctly addresses non-deterministic Go map iteration by collecting storage names, sorting them, and iterating in sorted order. This ensures consistent output across runs, which is essential for the parity tests between Go and TypeScript implementations.
testdata/sdl/v2.0/ip-endpoint/input.yaml (1)
1-49: LGTM!Well-structured SDL v2.0 test fixture for IP endpoint functionality. The configuration correctly demonstrates:
- Dual port exposure (80 and 443) with IP endpoint references
- Proper endpoint declaration matching the IP references in the service expose section
- Valid compute profile and placement configuration
testdata/sdl/v2.0/multiple-services/input.yaml (1)
1-93: LGTM!Comprehensive multi-service SDL v2.0 test fixture demonstrating:
- Service dependency chains (web → api → db)
- Multiple compute profiles with varying resource allocations
- Both
anyOfandallOfsignedBy constraints- Inter-service expose configurations
The hardcoded credentials (lines 29, 18) are appropriate for test fixtures.
testdata/sdl/invalid/invalid-port.yaml (1)
1-32: LGTM!Valid negative test fixture with intentionally invalid port value (99999 > 65535). This tests the port validation logic in
validateServicePortswhich enforcesport > 0 && port <= 65535.testdata/sdl/v2.1/credentials/input.yaml (1)
1-43: LGTM!Well-structured SDL v2.1 test fixture for private registry credentials. Correctly demonstrates the credentials block structure (host, email, username, password) for private container registry authentication.
ts/src/sdl/SDL/SDL.ts (3)
113-122: LGTM!Good addition of version field validation in
fromString. This catches malformed SDLs early and provides a clear error message.
834-864: LGTM!Good addition of deterministic storage key ordering via
.sort()in bothv2ManifestServiceParamsandv3ManifestServiceParams. This ensures consistent output for parity testing between Go and TypeScript implementations.
899-917: LGTM!Clean refactor of params handling. The conditional assignment (
if (params !== null)) prevents null params from polluting the manifest output.ts/src/sdl/SDL/parity.spec.ts (2)
24-79: LGTM!
makeSchemaLenientappropriately relaxes JSON Schema constraints for flexible parity testing. The recursive handling of nested structures (properties,items,anyOf,oneOf,allOf) is thorough.
303-315: Verify: v2.0 fixtures usev3Manifestmethod.The tests call
sdl.v3Manifest(true)for v2.0/beta2 fixtures instead ofsdl.v2Manifest(). The normalization handles GPU filtering for beta2, but verify this is the intended parity comparison approach.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (3)
specs/sdl/manifest.schema.yaml (1)
1-6: Consider adding standard JSON Schema root declarations.The schema starts directly with
items:without a root-leveltype: arraydeclaration or$schemaidentifier. While this may work with some validators, adding these improves compatibility and clarity:+$schema: "https://json-schema.org/draft/2020-12/schema" +type: array items: additionalProperties: false properties: name: type: stringts/src/sdl/SDL/parity.spec.ts (2)
68-76: Redundant processing for array items.Lines 68-76 re-check
additionalPropertiesandrequiredonitemsSchema, butmakeSchemaLenientwas already called onlenient.itemsat line 53, which would have handled these. This block is defensive but unnecessary.- if (lenient.type === "array" && lenient.items) { - const itemsSchema = lenient.items as Record<string, unknown>; - if (itemsSchema.additionalProperties === false) { - itemsSchema.additionalProperties = true; - } - if (itemsSchema.required && Array.isArray(itemsSchema.required)) { - delete itemsSchema.required; - } - } - return lenient;
81-101: Schema is recompiled on every validation call.
ajv.compile()is called each timevalidateAgainstSchemais invoked. Since the same schemas are validated multiple times across fixtures, consider caching compiled validators.+const schemaCache = new Map<string, ReturnType<typeof ajv.compile>>(); + function validateAgainstSchema(actual: unknown, schemaPath: string): { valid: boolean; errors: string[] } { if (!fs.existsSync(schemaPath)) { return { valid: true, errors: [] }; } + let validate = schemaCache.get(schemaPath); + if (!validate) { const schemaContent = fs.readFileSync(schemaPath, "utf8"); const schema = load(schemaContent) as JSONSchema; const lenientSchema = makeSchemaLenient(schema); - const validate = ajv.compile(lenientSchema as Record<string, unknown>); + validate = ajv.compile(lenientSchema as Record<string, unknown>); + schemaCache.set(schemaPath, validate); + } const valid = validate(actual);
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
specs/sdl/manifest.schema.yaml(1 hunks)ts/src/sdl/SDL/parity.spec.ts(1 hunks)
⏰ 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). (5)
- GitHub Check: proto
- GitHub Check: go
- GitHub Check: sdl-parity
- GitHub Check: test
- GitHub Check: coverage
🔇 Additional comments (7)
specs/sdl/manifest.schema.yaml (2)
295-314: Good use of$defsfor reusable credentials schema.The credentials schema is now properly defined once in
$defsand referenced via$refat lines 26 and 105, eliminating the previous duplication.
33-95: LGTM!The
exposeschema is well-structured with proper nesting forhttpOptions, required fields, and nullable patterns usinganyOfwithtype: 'null'.ts/src/sdl/SDL/parity.spec.ts (5)
103-171: LGTM!The
normalizeKeysfunction correctly handles key transformations and value normalization. The previous issue withfinalValuebeing overwritten has been addressed—now onlyfinalValueis assigned tonormalized[normalizedKey].
179-188: Numeric string normalization may have edge cases.The logic at lines 181-186 only normalizes strings containing
.and checks for specific trailing zero patterns. Integer strings (e.g.,"100") won't be converted, and strings like"1.0"might not match the.000000000000000000pattern.If this is intentional to handle specific Go/TS output differences, consider adding a brief comment explaining the expected scenarios.
251-280: LGTM!The
loadFixturesfunction correctly discovers and validates fixture directories, ensuring only complete fixtures (with input.yaml, manifest.json, and groups.json) are returned.
402-411: Invalid fixtures only tested against beta3.The invalid fixtures test uses hardcoded
"beta3"version. If v2.0/beta2 has different validation rules, consider also testing invalid fixtures against beta2, or documenting that invalid fixtures are version-specific.
295-329: LGTM!The v2.0 test suite correctly loads fixtures, normalizes both actual and expected outputs, compares them, and validates against schemas. The error logging on validation failure aids debugging.
| @@ -1,5 +1,5 @@ | |||
| /* eslint-disable @typescript-eslint/no-explicit-any */ | |||
There was a problem hiding this comment.
@stalniy I think this is the most important file to review, since it is related to the core implementation.
…sdl-parity-tests Signed-off-by: Artem Shcherbatiuk <[email protected]> Co-authored-by: Artem Shcherbatiuk <[email protected]>
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (7)
specs/sdl/sdl-input.schema.yaml (1)
386-388: Consider addingservicesto required fields.The
requiredarray includesversionanddeployment, butservicesis not required. Since deployments reference services by name, an SDL without services would likely fail at runtime. Addingserviceshere would catch this earlier during schema validation.required: - version - deployment + - servicests/src/sdl/SDL/SDL.ts (1)
1305-1313: Consider extracting duplicate amount formatting logic.This amount formatting logic is duplicated in both
v2Groups(lines 1305-1312) andv3Groups(lines 1210-1217). Consider extracting to a helper method:private formatAmount(amount: number | string | undefined): string { return typeof amount === "number" ? amount.toFixed(18) : (amount?.toString() || "0"); }specs/sdl/groups-output.schema.yaml (1)
1-5: Consider adding$schemadeclaration and metadata.Unlike
sdl-input.schema.yaml, this schema lacks the$schemadeclaration and descriptive metadata. Adding these would improve documentation and IDE support:$schema: http://json-schema.org/draft-07/schema# title: Akash SDL Groups Output Schema description: Schema for validating SDL groups output structure. items: # ... rest of schemaspecs/sdl/manifest-output.schema.yaml (1)
1-6: Consider adding$schemadeclaration and metadata.Similar to the groups output schema, adding metadata would improve documentation:
$schema: http://json-schema.org/draft-07/schema# title: Akash SDL Manifest Output Schema description: Schema for validating SDL manifest output structure. items: # ... rest of schemats/src/sdl/SDL/parity.spec.ts (3)
22-35: Consider caching compiled schema validators.
ajv.compile(schema)is called for each validation, recompiling the same schemas multiple times (once per fixture × 3 schemas). AJV caches compiled validators internally by schema identity, but since the schema is re-read and re-parsed each time, caching is bypassed.Consider preloading and compiling schemas once:
const schemaCache = new Map<string, AjvModule.ValidateFunction>(); function validateAgainstSchema(name: string, data: unknown, schemaPath: string): void { let validate = schemaCache.get(schemaPath); if (!validate) { const schemaContent = fs.readFileSync(schemaPath, "utf8"); const schema = load(schemaContent); validate = ajv.compile(schema as Record<string, unknown>); schemaCache.set(schemaPath, validate); } // ... rest of validation }
71-122: Consider parameterizing v2.0 and v2.1 test blocks.The two describe blocks are nearly identical. You could reduce duplication with parameterized tests:
describe.each([ { version: "v2.0", sdlVersion: "beta2" as const }, { version: "v2.1", sdlVersion: "beta3" as const }, ])("$version", ({ version, sdlVersion }) => { const fixtures = loadFixtures(version); fixtures.forEach((fixture) => { it(fixture.name, () => { // ... test body using sdlVersion }); }); });However, the current explicit form is also fine for readability.
124-142: Early return silently skips invalid SDL tests.If
testdata/sdl/invaliddoesn't exist, the describe block executes but defines no tests. This might hide a missing test fixture issue. Consider either:
- Making the directory required (throw an error)
- Using
describe.skipwith a message- At minimum, logging a warning
describe("invalid SDLs rejected", () => { const invalidDir = path.join(fixturesRoot, "invalid"); if (!fs.existsSync(invalidDir)) { it.skip("invalid fixtures directory not found", () => {}); return; } // ... });
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
go/sdl/parity_test.go(1 hunks)specs/sdl/README.md(1 hunks)specs/sdl/groups-output.schema.yaml(1 hunks)specs/sdl/manifest-output.schema.yaml(1 hunks)specs/sdl/sdl-input.schema.yaml(1 hunks)ts/src/sdl/SDL/SDL.ts(19 hunks)ts/src/sdl/SDL/parity.spec.ts(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- specs/sdl/README.md
🚧 Files skipped from review as they are similar to previous changes (1)
- go/sdl/parity_test.go
🧰 Additional context used
🧬 Code graph analysis (1)
ts/src/sdl/SDL/parity.spec.ts (1)
ts/src/sdl/SDL/SDL.ts (3)
validate(129-139)validate(318-333)SDL(89-1428)
🪛 GitHub Check: ts
ts/src/sdl/SDL/SDL.ts
[failure] 1212-1212:
Trailing spaces not allowed
[failure] 1307-1307:
Trailing spaces not allowed
⏰ 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). (5)
- GitHub Check: test
- GitHub Check: coverage
- GitHub Check: sdl-parity
- GitHub Check: go
- GitHub Check: proto
🔇 Additional comments (8)
specs/sdl/sdl-input.schema.yaml (2)
117-128: Verify the storage conditional validation logic.The second
if/thenblock appears to enforce that whenpersistentisfalse, theclassproperty must NOT exist. This seems inverted from the typical constraint (usually you'd want to restrict whenpersistent: truewithclass: ram). The first block (lines 105-116) already handles theclass: ramcase.Could you confirm this is the intended behavior? If it's meant to prevent non-persistent storage from having a class attribute, consider adding a comment explaining this constraint.
182-184: Verify denom pattern case sensitivity.The pattern
^(uakt|ibc/.*)$uses lowercaseibc. In the Cosmos ecosystem, IBC denoms are typically uppercase (e.g.,ibc/27394FB092D2ECCD...). If both cases are valid, consider making the pattern case-insensitive:- pattern: '^(uakt|ibc/.*)$' + pattern: '^(uakt|[iI][bB][cC]/.*)$'Or simply use uppercase if that's the canonical form.
ts/src/sdl/SDL/SDL.ts (3)
247-316: LGTM -convertToGoFormathandles key transformations comprehensively.The method handles Go-specific key naming conventions (snake_case, special cases like
HTTPOptions→httpOptions), empty array to null conversion forall_of/any_of, and properly skips undefined values. The recursion for arrays and nested objects is correct.
906-918: LGTM - Sorted storage keys ensure deterministic output.Sorting storage keys before iteration ensures consistent ordering between Go and TypeScript implementations, which is essential for parity testing.
1170-1179: LGTM - GPU default handling matches Go behavior.Always including GPU in groups output (defaulting to units
"0"when absent) ensures parity with Go's output format.specs/sdl/groups-output.schema.yaml (1)
49-50: LGTM - Schema structure is comprehensive.The groups output schema properly validates the nested structure including resources with CPU, GPU, memory, storage, and endpoints. The strict
additionalProperties: falseensures no unexpected fields pass validation.specs/sdl/manifest-output.schema.yaml (1)
295-314: LGTM - Good use of$defsfor reusable credentials schema.The credentials definition is properly extracted and referenced in two places, reducing duplication and ensuring consistency.
ts/src/sdl/SDL/parity.spec.ts (1)
37-69: LGTM - Fixture loading with helpful error messages.The
loadFixturesfunction provides clear error messages guiding developers to runmake generate-sdl-fixtureswhen fixtures are missing. This is good developer experience.
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (3)
ts/src/sdl/SDL/SDL.ts (3)
247-316: Consider a key transformation map for maintainability.The
convertToGoFormatmethod handles key transformations with a long chain of conditionals. While functional, this approach can be error-prone as the number of special cases grows. Lines 279 and 281 mapendpointSequenceNumberandexternalPortto themselves—verify these keys don't require snake_case transformation for Go compatibility.Consider replacing the conditional chain with a transformation map:
private readonly GO_KEY_MAP: Record<string, string> = { sequenceNumber: "sequence_number", signedBy: "signed_by", allOf: "all_of", anyOf: "any_of", quantity: "size", HTTPOptions: "httpOptions", IP: "ip" // add other mappings }; private convertToGoFormat(obj: unknown): unknown { // ... existing null/array/primitive checks ... for (const key in objRecord) { // ... existing undefined checks ... let convertedKey = this.GO_KEY_MAP[key] || key; // Then apply fallback transformations (camelCase, snake_case) if not in map // ... } }This makes the transformations explicit and easier to audit.
1028-1061: Update comment to reflect deduplication behavior.The comment at line 1029 states "Collect all IPs (including duplicates)" but line 1050 explicitly deduplicates via
new Set(endpointNames). The implementation correctly deduplicates, but the comment is misleading.Update the comment to:
- // Collect all IPs (including duplicates) to match Go behavior + // Collect all IPs from service endpoints, then deduplicate and sort for stable sequence assignment
1210-1218: Consider extracting amount formatting to reduce duplication.The amount formatting logic (lines 1210-1214) is duplicated in
v2Groups(lines 1305-1309). Extracting this to a helper method would improve maintainability and ensure consistent precision handling.Add a helper method:
private formatAmountForGo(amountValue: number | string | undefined): string { return typeof amountValue === "number" ? amountValue.toFixed(18) : (amountValue?.toString() || "0"); }Then replace both occurrences with:
const amountStr = this.formatAmountForGo(pricing.amount as number | string | undefined);
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
ts/src/sdl/SDL/SDL.ts(19 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
ts/src/sdl/SDL/SDL.ts (2)
ts/src/sdl/index.ts (1)
SdlValidationError(28-28)ts/src/sdl/types.ts (3)
v3Manifest(4-4)v3ComputeResources(218-224)v3DeploymentGroup(270-285)
⏰ 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). (5)
- GitHub Check: sdl-parity
- GitHub Check: coverage
- GitHub Check: test
- GitHub Check: proto
- GitHub Check: go
🔇 Additional comments (15)
ts/src/sdl/SDL/SDL.ts (15)
2-2: LGTM: Updated to named import from js-yaml.The change from
YAML.load(...)toload(...)aligns with modern js-yaml API usage.
57-57: LGTM: Clean type definition for output format switching.The
OutputFormattype provides clear intent for toggling between TypeScript and Go-compatible output formats.
129-139: LGTM: Updated YAML parsing to use load().The change aligns with the updated import and maintains existing validation logic.
318-333: LGTM: Extracted validation methods improve modularity.The new calls to
validateServiceImageandvalidateServicePortsmake the validation logic more organized and testable.
357-361: LGTM: Runtime validation ensures non-empty image names.This validation complements the JSON schema checks used in tests and provides runtime protection against empty or whitespace-only image names.
362-371: LGTM: Port range validation is correct.The validation ensures ports fall within the valid TCP/UDP range (1-65535), providing runtime protection consistent with the JSON schema constraints.
906-918: LGTM: Storage key sorting ensures deterministic output.Sorting storage keys before iteration ensures consistent, reproducible manifest output that matches Go implementation behavior.
920-936: LGTM: Proper null handling and deterministic storage ordering.The method correctly returns
nullfor undefined/null params and sorts storage keys for deterministic output. The error check at line 928 is appropriate for catching malformed configurations.
961-989: LGTM: Conditional params assignment matches Go's optional field handling.Only assigning
paramswhen non-null ensures the output matches Go's behavior of omitting optional fields that are null/undefined.
998-1010: LGTM: Format parameter enables Go-compatible output.The method cleanly supports both TypeScript and Go output formats by conditionally applying
convertToGoFormatand propagating the format parameter to dependent methods.
1080-1096: LGTM: Dynamic key naming supports format compatibility.Switching between
"quantity"and"size"based on theasStringparameter ensures the output matches expected formats for both TypeScript and manifest representations.
1099-1110: LGTM: Consistent key switching for storage units.The dynamic key naming matches the pattern used in
resourceUnitMemoryand ensures format compatibility.
1170-1179: LGTM: GPU always included to match Go behavior.The code correctly defaults to GPU with
units: "0"when not present, ensuring output parity with the Go implementation. The comment clearly explains this design decision.
1194-1287: LGTM: Format parameter and precision handling ensure Go parity.The method correctly handles the format parameter and applies 18-decimal precision to amounts (lines 1212-1214), which is critical for matching Go's numeric output. The
convertToGoFormatconversion is appropriately applied at the end.
1190-1192: Thegroups()method passes theformatparameter only tov3Groups(), notv2Groups()(line 1191).This is a factual difference in how the two code paths handle the format parameter. If v2 is deprecated and no longer in active use, this design is appropriate. However, if v2 deployments are still supported or tested for parity with v3, the v2Groups() method should also accept and use the format parameter to maintain feature parity.
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (2)
go/sdl/schema_validator.go (1)
114-120: Consider usingerrors.Joinor structured error for validation failures.The current approach concatenates error strings into a slice and formats with
%v. This works but loses structure for downstream error handling.+import "strings" + if !result.Valid() { - var errors []string + var errMsgs []string for _, desc := range result.Errors() { - errors = append(errors, desc.String()) + errMsgs = append(errMsgs, desc.String()) } - return fmt.Errorf("schema validation failed: %v", errors) + return fmt.Errorf("schema validation failed: %s", strings.Join(errMsgs, "; ")) }ts/src/sdl/SDL/SDL.ts (1)
276-281: Redundant key mappings preserve original values unnecessarily.Lines 278-281 map keys to themselves (
sequenceNumber→sequenceNumber,externalPort→externalPort), which has no effect. These could be removed unless they're placeholders for future mappings.if (key === "sequenceNumber") { convertedKey = "sequence_number"; - } else if (key === "endpointSequenceNumber") { - convertedKey = "endpointSequenceNumber"; - } else if (key === "externalPort") { - convertedKey = "externalPort"; } else if (key === "signedBy") {If these are intentionally preserved to document "no change needed", consider adding a comment.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
go/sdl/cmd/generate-sdl-fixtures/main.go(1 hunks)go/sdl/parity_test.go(1 hunks)go/sdl/schema_validator.go(1 hunks)go/sdl/sdl.go(2 hunks)ts/src/sdl/SDL/SDL.ts(19 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- go/sdl/cmd/generate-sdl-fixtures/main.go
- go/sdl/parity_test.go
🧰 Additional context used
🧬 Code graph analysis (2)
ts/src/sdl/SDL/SDL.ts (1)
ts/src/sdl/types.ts (4)
v3Sdl(100-105)v3Manifest(4-4)v3ComputeResources(218-224)v3DeploymentGroup(270-285)
go/sdl/schema_validator.go (1)
go/sdl/sdl.go (1)
ReadFile(83-89)
⏰ 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). (5)
- GitHub Check: sdl-parity
- GitHub Check: test
- GitHub Check: coverage
- GitHub Check: go
- GitHub Check: proto
🔇 Additional comments (6)
go/sdl/sdl.go (1)
92-136: Schema validation integration is consistent and well-structured.The pattern of capturing
schemaErrupfront and callingcheckSchemaValidationResultat each exit point (both error and success) provides comprehensive coverage for comparing schema validation results against Go's runtime validation. All error paths are covered.One consideration: since
schemaErris captured once at the start butcheckSchemaValidationResultis called at different validation stages, any logged mismatch may not distinguish which validation stage disagreed with the schema. This is likely acceptable for parity testing purposes, but worth noting if debugging becomes difficult.ts/src/sdl/SDL/SDL.ts (5)
362-371: Port validation logic is correct.The validation correctly ensures ports are in the valid range (1-65535). The error message accurately describes the constraint.
1170-1179: GPU default handling ensures Go parity.The logic to always include GPU with a default
units: { val: "0" }when not present ensures consistent output structure matching Go's behavior. The comment is helpful.
906-936: Sorted storage keys ensure deterministic manifest output.The changes to sort storage keys (
Object.keys(params?.storage ?? {}).sort()) in both v2 and v3 manifest service params functions ensure consistent ordering across runs, which is essential for parity testing.
1053-1058: Verify if duplicate IP overwrite behavior matches Go implementation.The code collects all IPs including duplicates (line 39) but assigns sequence numbers via object key assignment (line 1057), which means duplicate IPs will have only their last sequence number retained. The comments indicate this matches Go behavior, but this should be confirmed against the Go implementation to ensure the overwrite semantics are intentional rather than a bug.
1210-1217:toFixed(18)with very large numbers can produce scientific notation, but pricing amounts in fixtures are well within safe range.JavaScript's
toFixed(18)does produce scientific notation for values ≥ 1e21 (e.g.,(1e21).toFixed(18)returns"1e+21"), but all pricing amounts in SDL fixtures are substantially smaller (ranging from 25 to 1,000,000), making this a non-issue in practice. The current implementation is safe for the actual use cases in this codebase.
| type noOpLogger struct{} | ||
|
|
||
| func (l *noOpLogger) Debug(_ string, _ ...any) {} | ||
| func (l *noOpLogger) Info(_ string, _ ...any) {} | ||
| func (l *noOpLogger) Warn(_ string, _ ...any) {} | ||
| func (l *noOpLogger) Error(_ string, _ ...any) {} | ||
| func (l *noOpLogger) With(_ ...any) log.Logger { return l } | ||
| func (l *noOpLogger) Impl() any { return l } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, search for the log.Logger interface definition in the repository
rg -n "type Logger interface" --type goRepository: akash-network/chain-sdk
Length of output: 49
🏁 Script executed:
# Check for explicit interface satisfaction checks in the schema_validator.go file
rg -n "var _ " go/sdl/schema_validator.goRepository: akash-network/chain-sdk
Length of output: 49
🏁 Script executed:
# Also search in the go directory for any imports of cosmossdk/log
rg -n "cosmossdk/log\|\"log\"" go/sdl/schema_validator.goRepository: akash-network/chain-sdk
Length of output: 49
🏁 Script executed:
# Read the schema_validator.go file to see imports and context
head -30 go/sdl/schema_validator.goRepository: akash-network/chain-sdk
Length of output: 671
🏁 Script executed:
# Look for other logger implementations in the codebase
rg -n "type.*Logger.*struct" --type goRepository: akash-network/chain-sdk
Length of output: 175
🏁 Script executed:
# Check if there's documentation or test files that use the noOpLogger
rg -n "noOpLogger" --type goRepository: akash-network/chain-sdk
Length of output: 675
🏁 Script executed:
# Check go.mod for cosmossdk/log dependency
rg "cosmossdk" go.mod || rg "log" go.mod | head -20Repository: akash-network/chain-sdk
Length of output: 160
🏁 Script executed:
# Let's check if there's a go.mod in the root or parent directory
find . -name "go.mod" -type f | head -5Repository: akash-network/chain-sdk
Length of output: 143
🏁 Script executed:
# Check the dummyLogger implementation for comparison
sed -n '25,35p' go/util/ctxlog/ctxlog.goRepository: akash-network/chain-sdk
Length of output: 254
🏁 Script executed:
# Look at more context around noOpLogger to see if there's an explicit interface assertion
sed -n '78,92p' go/sdl/schema_validator.goRepository: akash-network/chain-sdk
Length of output: 528
Add explicit interface assertion to noOpLogger for compile-time safety.
While noOpLogger is functional and satisfies the cosmossdk.io/log.Logger interface (as evidenced by its use in the log.Logger typed field), it lacks the explicit compile-time interface check present in the similar dummyLogger implementation in go/util/ctxlog/ctxlog.go. Add var _ log.Logger = (*noOpLogger)(nil) after the struct definition to provide compile-time verification of interface compliance, consistent with the codebase pattern.
🤖 Prompt for AI Agents
In go/sdl/schema_validator.go around lines 80 to 87, add an explicit
compile-time interface assertion for noOpLogger to ensure it implements
cosmossdk.io/log.Logger: after the noOpLogger methods/struct block, insert a
declaration that assigns a nil *noOpLogger to a log.Logger variable (var _
log.Logger = (*noOpLogger)(nil)) so the compiler verifies interface compliance
consistent with the dummyLogger pattern elsewhere.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
go/sdl/cmd/generate-sdl-fixtures/main.go (2)
13-15: Make fixtures root and version discovery less cwd‑dependent
fixturesRoot := "../../testdata/sdl"and the hard‑codedversions := []string{"v2.0", "v2.1"}assume a specific working directory and require code edits when new versions are added.Consider:
- Deriving
fixturesRootrelative to the executable (or accepting it as a flag/env), sogo run ./go/sdl/cmd/generate-sdl-fixturesfrom repo root still works reliably.- Optionally discovering versions by listing subdirectories under
fixturesRootinstead of keeping a manual slice, unless you explicitly want to gate which versions are generated.
49-81: Permissions and overall generation loop look good, with a minor mode nitThe manifest/groups generation flow and error handling look solid and idempotent: each failure is logged and the loop continues with other fixtures.
One minor nit:
os.WriteFile(..., 0600)will create new JSON fixtures as user‑read/write only. For repo fixtures that are meant to be shared,0644is more typical; consider switching to that unless you intentionally want stricter permissions.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
go/provider/client/client.go(1 hunks)go/sdl/cmd/generate-sdl-fixtures/main.go(1 hunks)go/sdl/go.mod(2 hunks)go/util/jwt/jwt_test.go(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
go/sdl/cmd/generate-sdl-fixtures/main.go (2)
go/sdl/sdl.go (1)
ReadFile(83-89)go/testutil/deployment.go (1)
DeploymentGroups(54-62)
⏰ 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). (5)
- GitHub Check: proto
- GitHub Check: test
- GitHub Check: coverage
- GitHub Check: sdl-parity
- GitHub Check: go
🔇 Additional comments (3)
go/provider/client/client.go (1)
99-103: LGTM!Clean stylistic improvement. Omitting names for unused receiver and parameters is idiomatic Go and equivalent to using
_placeholders.go/util/jwt/jwt_test.go (1)
189-189: LGTM! Clean simplification.The variadic append is more idiomatic and concise than the previous nested loop approach while preserving identical behavior.
go/sdl/go.mod (1)
8-8: Consider the maintenance posture of gojsonschema and its dependencies.The
gojsonschema v1.2.0dependency is Go 1.25-compatible. However, the library hasn't had a release since October 2019 (5+ years), and its transitive dependencies (gojsonpointerandgojsonreference) were last maintained in January 2018 and are now effectively abandoned. While no CVEs are published, the lack of maintenance creates a future security posture risk. If production use requires more actively-maintained alternatives, consider evaluating other JSON schema validators likegoogle/jsonschema-goorsanthosh-tekuri/jsonschema.Also applies to: 13-13, 166-167
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
go/cli/go.sumis excluded by!**/*.sum
📒 Files selected for processing (1)
go/cli/go.mod(1 hunks)
⏰ 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). (5)
- GitHub Check: go
- GitHub Check: proto
- GitHub Check: sdl-parity
- GitHub Check: coverage
- GitHub Check: test
| github.com/xeipuuv/gojsonpointer v0.0.0-20180127040702-4e3ac2762d5f // indirect | ||
| github.com/xeipuuv/gojsonreference v0.0.0-20180127040603-bd5ef7bd5415 // indirect | ||
| github.com/xeipuuv/gojsonschema v1.2.0 // indirect |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check if these packages are directly imported in go/cli code
echo "=== Checking direct imports in go/cli ==="
find go/cli -name '*.go' -type f ! -path '*/vendor/*' ! -path '*/.git/*' 2>/dev/null | xargs grep -l 'xeipuuv' 2>/dev/null || echo "No direct imports found"
# Check if they're used in go/sdl instead
echo -e "\n=== Checking imports in go/sdl ==="
find go/sdl -name '*.go' -type f ! -path '*/vendor/*' ! -path '*/.git/*' 2>/dev/null | xargs grep -l 'xeipuuv' 2>/dev/null || echo "No direct imports found"
# Verify current versions in go/sdl/go.mod
echo -e "\n=== Checking go/sdl/go.mod for these packages ==="
grep -E '(gojsonpointer|gojsonreference|gojsonschema)' go/sdl/go.mod 2>/dev/null || echo "Not found in go/sdl/go.mod"
# Check if go/cli/go.mod marks them as indirect
echo -e "\n=== Checking indirect status in go/cli/go.mod ==="
grep -E '(gojsonpointer|gojsonreference|gojsonschema)' go/cli/go.mod | head -5Repository: akash-network/chain-sdk
Length of output: 739
🏁 Script executed:
#!/bin/bash
# Check if these packages have known security issues
echo "=== Researching package maintenance status ==="
echo "gojsonschema and dependencies:"
echo "- gojsonpointer v0.0.0-20180127... (2018 pseudo-version)"
echo "- gojsonreference v0.0.0-20180127... (2018 pseudo-version)"
echo "- gojsonschema v1.2.0"
# Check go/cli/go.mod structure to understand why these were added
echo -e "\n=== Checking go/cli/go.mod context ==="
wc -l go/cli/go.mod
echo -e "\n=== Lines around 215-217 (the dependencies in question) ==="
sed -n '210,220p' go/cli/go.mod
# Check if go/sdl is a dependency of go/cli
echo -e "\n=== Checking if go/sdl is a direct dependency of go/cli ==="
grep 'github.com/akash-network/akash' go/cli/go.mod | head -5Repository: akash-network/chain-sdk
Length of output: 1056
🏁 Script executed:
#!/bin/bash
# Find all akash-network dependencies in go/cli/go.mod
echo "=== All akash-network dependencies in go/cli/go.mod ==="
grep 'github.com/akash-network' go/cli/go.mod
# Check the requires section specifically
echo -e "\n=== go/cli require section ==="
sed -n '/^require/,/^)/p' go/cli/go.mod | head -20
# Check if go/sdl/go.mod is in the workspace (if go 1.18+)
echo -e "\n=== Checking for go.work file ==="
if [ -f go.work ]; then
echo "Found go.work:"
cat go.work
else
echo "No go.work file found"
fi
# Verify go.mod version
echo -e "\n=== Go version in go/cli/go.mod ==="
head -1 go/cli/go.modRepository: akash-network/chain-sdk
Length of output: 1061
🏁 Script executed:
#!/bin/bash
# Double-check: search ALL go/cli .go files for any xeipuuv imports (not just find | grep)
echo "=== Direct search for xeipuuv in all go/cli .go files ==="
find go/cli -type f -name '*.go' -exec grep -H 'xeipuuv\|gojsonschema\|gojsonpointer\|gojsonreference' {} \; 2>/dev/null || echo "No matches found"
# Check if go/cli imports go/sdl anywhere
echo -e "\n=== Checking if go/cli imports go/sdl (any variant) ==="
find go/cli -type f -name '*.go' -exec grep -H 'pkg\.akt\.dev/go/sdl\|akash-network.*sdl\|".*sdl' {} \; 2>/dev/null || echo "No sdl imports found"
# List all direct Go files in go/cli (not tests, not vendor)
echo -e "\n=== Go source files in go/cli ==="
find go/cli -type f -name '*.go' ! -path '*/vendor/*' ! -path '*/.git/*' | head -20Repository: akash-network/chain-sdk
Length of output: 849
🏁 Script executed:
#!/bin/bash
# Check if go/sdl/go.mod lists go/cli as a dependency (check dependency chain)
echo "=== Checking if go/sdl depends on go/cli ==="
grep 'go/cli\|pkg\.akt\.dev/go/cli' go/sdl/go.mod 2>/dev/null || echo "No go/cli dependency found"
# Check go/cli/go.mod to see if it explicitly requires go/sdl
echo -e "\n=== Checking if go/cli/go.mod requires go/sdl ==="
grep -A 200 '^require' go/cli/go.mod | grep 'go/sdl\|pkg\.akt\.dev/go/sdl' || echo "go/sdl not in explicit requires"
# Check the full require block to understand the structure
echo -e "\n=== Full require block of go/cli/go.mod ==="
sed -n '/^require (/,/^)/p' go/cli/go.mod | tail -10Repository: akash-network/chain-sdk
Length of output: 705
Remove gojsonschema dependencies from go/cli/go.mod—they're transitive from go/sdl.
These three packages are only directly used in go/sdl (schema_validator.go, parity_test.go), not in go/cli. Since go/cli/go.mod explicitly requires go/sdl with a local replace (pkg.akt.dev/go/sdl => ../sdl), the xeipuuv packages should be pulled in transitively and don't need explicit declarations in go/cli/go.mod.
Remove lines 215-217 from go/cli/go.mod; go mod tidy will re-add them only if necessary. The old package versions (2018 pseudo-versions) are not a concern since they're already locked in go/sdl/go.mod where they're actually used.
🤖 Prompt for AI Agents
In go/cli/go.mod around lines 215 to 217, the explicit dependencies on
github.com/xeipuuv/gojsonpointer, github.com/xeipuuv/gojsonreference, and
github.com/xeipuuv/gojsonschema are unnecessary because they are transitive
through the local replace of pkg.akt.dev/go/sdl => ../sdl; remove those three
lines and then run "go mod tidy" in go/cli to refresh module requirements so
only needed dependencies remain.
📝 Description
I added parity tests between
the Golang and TypeScript SDL implementations.
I defined YAML schemas for groups, manifest, and SDL input. I use these schemas in tests where I pass tenant input into the Golang and TypeScript implementations and compare the output with expected values, using the defined schemas.
Additionally, I ensured that the IDE supports YAML validation against the schema when creating manifests in the editor.
Important note:
I generated input and output for tests using AI, let me know if it makes sense for you.
🔧 Purpose of the Change
📌 Related Issues
✅ Checklist