Minimal modelling to simply get started#3
Merged
petrubraha merged 21 commits intomainfrom Mar 7, 2026
Merged
Conversation
Member
Author
|
Install smithy cli https://smithy.io/2.0/guides/smithy-cli/cli_installation.html. Would recommend use more options from here https://smithy.io/2.0/guides/smithy-build-json.html#projections. The generation can be triggered with the following commands: |
petrubraha
added
documentation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughAdds a Smithy-based API project "InAndOut": build configuration and .gitignore, core UUID types, error shapes, a REST/JSON service declaration, and a Travelling Salesman Problem (TSP) operation with its input/output and domain types. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 10
🧹 Nitpick comments (2)
README.md (1)
3-11: Add a minimal “how to generate” section for first-run onboarding.Given the PR goal (“simply get started”), Line 3 onward should include runnable generation commands in addition to links.
Suggested README patch
Complete API model for our system. + +## Quick start + +```bash +smithy build --projection openapi-spec +smithy build --projection typescript-client +```🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@README.md` around lines 3 - 11, Add a minimal "How to generate" section to the README starting after the existing header (line 3 onward) that provides runnable first-run commands; include the smithy commands suggested (e.g., smithy build --projection openapi-spec and smithy build --projection typescript-client), a short note about required prerequisites (Smithy installed and project root), and a one-liner example showing where outputs land so new contributors can run generation immediately without following external links.smithy-build.json (1)
31-37: Add explicitserviceproperty totypescript-codegenplugin.The
typescript-codegenplugin should explicitly set theserviceproperty for consistency with other projections (openapi and java-client) and to ensure deterministic generation. While Smithy's TypeScript codegen auto-infers the service when exactly one exists, explicitly setting it provides safer future proofing if additional services are added to the model.Suggested patch
"typescript-client": { "plugins": { "typescript-codegen": { + "service": "com.shopping.inandout#InAndOut", "package": "@inandout/client", "packageVersion": "0.0.1" } } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@smithy-build.json` around lines 31 - 37, Add an explicit "service" property to the "typescript-codegen" plugin configuration in the smithy-build.json so the plugin no longer relies on auto-inference; update the "typescript-codegen" block (the object under "typescript-client" -> "plugins" -> "typescript-codegen") to include "service": "YourServiceName" (replace with the actual Smithy service ID from the model) alongside "package" and "packageVersion" to ensure deterministic generation.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@build.gradle.kts`:
- Around line 32-35: The repositories block is declared after dependencies which
can break dependency resolution; move the repositories { mavenLocal();
mavenCentral() } block so it appears before any dependencies { ... }
declarations (including the other occurrence noted) so Gradle knows where to
fetch artifacts when evaluating the dependencies blocks; ensure every
repositories block in the file is placed above its corresponding dependencies
block.
- Around line 8-11: The build currently pins smithy codegen and client-core to
version "0.0.3" using smithyBuild(...) and implementation(...); verify that
"software.amazon.smithy.java:client-codegen:0.0.3" and
"software.amazon.smithy.java:client-core:0.0.3" are acceptable for your
stability requirements and, if a more stable or newer release is required,
update those version strings (in the smithyBuild(...) and implementation(...)
declarations) to the vetted release version or to a project version property so
you can manage upgrades centrally.
In `@gradle.properties`:
- Around line 1-2: Remove the unused gradle property by deleting the orphaned
smithyVersion entry from gradle.properties and then align Smithy artifact
versions by updating smithy-build.json to reference the same Smithy core version
used elsewhere (e.g., change smithy-build.json smithy artifact versions to
1.68.0) or, alternatively, add a comment in smithy-build.json documenting the
intentional version difference; ensure build.gradle.kts is not relying on
smithyVersion before removal and update any references if present.
In `@gradle/wrapper/gradle-wrapper.properties`:
- Around line 3-6: Add a distributionSha256Sum entry next to the existing
distributionUrl property to enable download integrity verification: compute or
obtain the SHA-256 checksum for the gradle-8.2.1-bin.zip (matching
distributionUrl) from the official Gradle distribution metadata or by hashing
the ZIP, then add a distributionSha256Sum=<sha256> property alongside
distributionUrl, keeping networkTimeout, zipStoreBase and zipStorePath
unchanged; ensure the checksum string exactly matches the ZIP file for
gradle-8.2.1.
In `@settings.gradle.kts`:
- Around line 7-11: The unconditional mavenLocal() call in the
pluginManagement.repositories (repositories block) causes machine-specific
plugin resolution; remove mavenLocal() or make it opt-in by checking an
environment variable (e.g., USE_MAVEN_LOCAL) before adding mavenLocal() so CI
and other developers use consistent Maven Central/Gradle Plugin Portal
resolution. Update the repositories block to either drop the mavenLocal() entry
entirely or wrap its inclusion behind an env-var check so only developers
explicitly opting in will use the local Maven cache.
In `@smithy-build.json`:
- Around line 23-29: The smithy projection config defines a "java-client" plugin
"java-client-codegen" but the maven.dependencies array is missing the
corresponding Smithy Java client-codegen artifact; add the dependency
"software.amazon.smithy.java:client-codegen:0.0.3" to the maven.dependencies so
the "java-client-codegen" plugin will have its required Maven artifact (ensure
the group:artifact:version matches exactly and aligns with the entry referenced
in build.gradle.kts).
In `@smithy/model/common.smithy`:
- Around line 5-7: The UUID string constraint is wrong: the `@length`(min: 32,
max: 32) conflicts with the hyphenated `@pattern` and should be corrected; update
the definition of UUID (the string UUID type with the `@pattern`) to either remove
the `@length` constraint entirely or change it to `@length`(min: 36, max: 36) so it
matches hyphenated UUIDs (e.g., 550e8400-e29b-41d4-a716-446655440000).
In `@smithy/model/main.smithy`:
- Around line 9-14: The service-level `@paginated` trait on the InAndOut service
references pagination fields (nextToken, pageSize, tokens) that do not exist on
the operation shapes; update either the trait or the I/O shapes: remove the
`@paginated` trait if FindTspSolution is not paginated, or add the required fields
to the FindTspSolutionInput and FindTspSolutionOutput structures (in
tsp-io.smithy) — add nextToken and pageSize to FindTspSolutionInput and add
tokens (rename soltuionList to tokens or map its contents) and nextToken to
FindTspSolutionOutput so the names match the `@paginated` parameters.
In `@smithy/model/tsp/tsp-io.smithy`:
- Around line 12-15: Update the misspelled field and type in the
FindTspSolutionOutput structure: rename the field soltuionList to solutionList
and its type SoltuionList to SolutionList in tsp-io.smithy; then update
tsp-types.smithy to rename the Soltuion shape to Solution and SoltuionList to
SolutionList so generated classes/fields (Solution, SolutionList, solutionList)
use the correct spelling and stay consistent across the API.
In `@smithy/model/tsp/tsp-types.smithy`:
- Around line 8-16: Rename the misspelled Smithy types: change the structure
name Soltuion to Solution and the list type SoltuionList to SolutionList,
keeping the field nodeIdList and its UUIDList type unchanged; update any
references/usages of Soltuion and SoltuionList in the model or codegen inputs to
the corrected names so generated code and imports reflect Solution and
SolutionList.
---
Nitpick comments:
In `@README.md`:
- Around line 3-11: Add a minimal "How to generate" section to the README
starting after the existing header (line 3 onward) that provides runnable
first-run commands; include the smithy commands suggested (e.g., smithy build
--projection openapi-spec and smithy build --projection typescript-client), a
short note about required prerequisites (Smithy installed and project root), and
a one-liner example showing where outputs land so new contributors can run
generation immediately without following external links.
In `@smithy-build.json`:
- Around line 31-37: Add an explicit "service" property to the
"typescript-codegen" plugin configuration in the smithy-build.json so the plugin
no longer relies on auto-inference; update the "typescript-codegen" block (the
object under "typescript-client" -> "plugins" -> "typescript-codegen") to
include "service": "YourServiceName" (replace with the actual Smithy service ID
from the model) alongside "package" and "packageVersion" to ensure deterministic
generation.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 7f09ca8a-8994-45f6-85ec-6865da1ce85d
⛔ Files ignored due to path filters (1)
gradle/wrapper/gradle-wrapper.jaris excluded by!**/*.jar
📒 Files selected for processing (16)
.gitattributes.gitignoreREADME.mdbuild.gradle.ktsgradle.propertiesgradle/wrapper/gradle-wrapper.propertiesgradlewgradlew.batsettings.gradle.ktssmithy-build.jsonsmithy/model/common.smithysmithy/model/errors.smithysmithy/model/main.smithysmithy/model/tsp/tsp-apis.smithysmithy/model/tsp/tsp-io.smithysmithy/model/tsp/tsp-types.smithy
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
gradle/wrapper/gradle-wrapper.properties (1)
3-4:⚠️ Potential issue | 🟠 MajorAdd Gradle wrapper checksum verification.
Line 3 sets the distribution URL, but
distributionSha256Sumis still missing. This leaves wrapper downloads unverified.🔧 Suggested patch
distributionUrl=https\://services.gradle.org/distributions/gradle-8.2.1-bin.zip +distributionSha256Sum=03ec176d388f2aa99defcadc3ac6adf8dd2bce5145a129659537c0874dea5ad1 networkTimeout=10000Use this to verify expected checksum vs file configuration:
#!/bin/bash set -euo pipefail PROP_FILE="gradle/wrapper/gradle-wrapper.properties" echo "== distributionUrl ==" rg -n '^distributionUrl=' "$PROP_FILE" echo "== Official checksum for gradle-8.2.1-bin.zip ==" EXPECTED_SHA="$(curl -fsSL https://services.gradle.org/distributions/gradle-8.2.1-bin.zip.sha256 | tr -d '\r\n')" echo "$EXPECTED_SHA" echo "== Configured distributionSha256Sum (if any) ==" rg -n '^distributionSha256Sum=' "$PROP_FILE" || true🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@gradle/wrapper/gradle-wrapper.properties` around lines 3 - 4, Add checksum verification by adding the distributionSha256Sum property to gradle-wrapper.properties to match the Gradle distribution set in distributionUrl; fetch the official SHA-256 for gradle-8.2.1-bin.zip and add a line distributionSha256Sum=<EXPECTED_SHA> alongside the existing distributionUrl and networkTimeout entries so the Gradle wrapper validates the downloaded archive.
🧹 Nitpick comments (1)
README.md (1)
5-11: Consider adding usage instructions to the README.The PR comments mention useful commands like
smithy build --projection openapi-specandsmithy build --projection typescript-client. Including these in the README would improve developer onboarding.Also, the Google Doc link (line 7) may not be accessible to all contributors—consider summarizing key points or noting access requirements.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@README.md` around lines 5 - 11, Add a short "Usage / Developer onboarding" section to README that lists the key smithy commands (e.g., `smithy build --projection openapi-spec` and `smithy build --projection typescript-client`) with one-line descriptions of what each does, and include example invocation notes; also replace or augment the Google Doc link with a brief summary of its critical points or an "Access required" note so contributors know the doc may be private and what to request if they need access.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@smithy/model/common.smithy`:
- Around line 5-6: The `@pattern` annotation for the UUID type currently restricts
hex letters to lowercase
("^[0-9a-f]{8}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{12}$"); update this
pattern to accept both cases (e.g., use [0-9a-fA-F] for each hex group or a
case-insensitive flag) so uppercase UUIDs are allowed, and keep the `@length`(min:
36, max: 36) unchanged; modify the pattern in common.smithy where the `@pattern`
is declared.
---
Duplicate comments:
In `@gradle/wrapper/gradle-wrapper.properties`:
- Around line 3-4: Add checksum verification by adding the distributionSha256Sum
property to gradle-wrapper.properties to match the Gradle distribution set in
distributionUrl; fetch the official SHA-256 for gradle-8.2.1-bin.zip and add a
line distributionSha256Sum=<EXPECTED_SHA> alongside the existing distributionUrl
and networkTimeout entries so the Gradle wrapper validates the downloaded
archive.
---
Nitpick comments:
In `@README.md`:
- Around line 5-11: Add a short "Usage / Developer onboarding" section to README
that lists the key smithy commands (e.g., `smithy build --projection
openapi-spec` and `smithy build --projection typescript-client`) with one-line
descriptions of what each does, and include example invocation notes; also
replace or augment the Google Doc link with a brief summary of its critical
points or an "Access required" note so contributors know the doc may be private
and what to request if they need access.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: d4f9f702-9b1f-44bd-ba8c-7c176914f02a
⛔ Files ignored due to path filters (1)
gradle/wrapper/gradle-wrapper.jaris excluded by!**/*.jar
📒 Files selected for processing (16)
.gitattributes.gitignoreREADME.mdbuild.gradle.ktsgradle.propertiesgradle/wrapper/gradle-wrapper.propertiesgradlewgradlew.batsettings.gradle.ktssmithy-build.jsonsmithy/model/common.smithysmithy/model/errors.smithysmithy/model/main.smithysmithy/model/tsp/tsp-apis.smithysmithy/model/tsp/tsp-io.smithysmithy/model/tsp/tsp-types.smithy
lefcos
approved these changes
Mar 7, 2026
narenkarthik370h
approved these changes
Mar 7, 2026
narenkarthik370h
left a comment
narenkarthik370h
left a comment
There was a problem hiding this comment.
everything's decent for me kudos to you...let's build more and see whether things are working or not...approved from my side
narenkarthik370h
approved these changes
Mar 7, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
One single operation needed to spin up the development. I was stuck in too many models with the previous PR.
Functional goals:
Feel free to review the current code and add comments where problems could arise.
Summary by CodeRabbit
New Features
Documentation
Chores