Improve golang type documentation and examples #631
Conversation
|
Most of this came from diffing the specs with: # Start the server
make dev-compose
# Wait for server to be ready, then fetch and compare
curl -s http://localhost:8080/openapi.yaml | yq -o json | jq -S > /tmp/localhost-openapi.json
yq -o json docs/reference/api/openapi.yaml | jq -S > /tmp/docs-openapi.json
diff -u /tmp/docs-openapi.json /tmp/localhost-openapi.json > schemas.diffAnd then going through and trying to align these as much as possible |
domdomegg
force-pushed
the
adamj/improve-golang-type-docs
branch
2 times, most recently
from
5246f8d to
3ee0672
Compare
|
I have no idea why go is panicking now: https://github.com/modelcontextprotocol/registry/actions/runs/18327735919/job/52196150090?pr=631 Stacktrace not very helpful, and the changes really don't seem like they would cause this. |
rdimitrov
dismissed
their stale review
I had this opened on the file changes and missed the failing checks
Enhanced Go struct tags throughout pkg/api/v0/types.go and pkg/model/types.go to better align with the OpenAPI specification: - Added comprehensive doc tags with detailed field descriptions - Added example tags showing valid values for fields - Added format tags (uri, date-time) for better type specification - Added pattern and enum tags for validation constraints - Added placeholder field to Input struct matching OpenAPI spec - Removed redundant comments in favor of inline struct tags These changes improve code clarity and make the types more self-documenting, while maintaining compatibility with the OpenAPI schema definitions.
Updated test expectations to align with actual Huma framework behavior where schema validation occurs before handler execution: - Changed expected status from 400 to 422 (Unprocessable Entity) - Updated error message checks to match Huma's pattern validation error - Fixed "invalid token" test to use valid server name format Schema validation in Huma happens at the framework level before the handler function runs, so invalid server names (with multiple slashes, no slashes, etc.) return 422 with pattern matching errors rather than custom 400 errors. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]>
rdimitrov
force-pushed
the
adamj/improve-golang-type-docs
branch
2 times, most recently
from
9e61fb5 to
0ef6cf4
Compare
…itted flags Signed-off-by: Radoslav Dimitrov <[email protected]>
rdimitrov
force-pushed
the
adamj/improve-golang-type-docs
branch
from
0ef6cf4 to
85a7051
Compare
I hope you don't mind I tried fixing this and from what I found it's caused by Huma which apparently panics if both For the sake of the tests, I've removed the default labels for the failing properties (IsRequired, IsSecret and IsRepeated, included in my last commit), but this is still present in the reference openapi spec, so we have to decide if we keep it or come with a workaround for this. |
<!-- Provide a brief summary of your changes --> ## Motivation and Context <!-- Why is this change needed? What problem does it solve? --> While debugging #631 noticed the cache was weirdly implemented, so the following PR updates the caching strategy across a few of our CI workflows. - Replaced manual actions/cache with the built-in `cache: true` of setup-go - Switched from manual golangci-lint installation to their official golangci-lint-action - Removed redundant go mod download commands - Added cache to the deployment pipeline for both staging and production (we should see faster Pulumi deployments) - Same for the release workflow so hopefully faster release builds ## How Has This Been Tested? <!-- Have you tested this in a real application? Which scenarios were tested? --> ## Breaking Changes <!-- Will users need to update their code or configurations? --> No ## Types of changes <!-- What types of changes does your code introduce? Put an `x` in all the boxes that apply: --> - [ ] Bug fix (non-breaking change which fixes an issue) - [ ] New feature (non-breaking change which adds functionality) - [ ] Breaking change (fix or feature that would cause existing functionality to change) - [ ] Documentation update ## Checklist <!-- Go over all the following points, and put an `x` in all the boxes that apply. --> - [ ] I have read the [MCP Documentation](https://modelcontextprotocol.io) - [ ] My code follows the repository's style guidelines - [ ] New and existing tests pass locally - [ ] I have added appropriate error handling - [ ] I have added or updated documentation as needed ## Additional context <!-- Add any other context, implementation notes, or design decisions --> Signed-off-by: Radoslav Dimitrov <[email protected]>
Sounds good, thanks for fixing! Maybe we should report it upstream to huma as well.
I think it's fine if there's a little divergence - at least this PR brings them much closer. |
|
@domdomegg - in light of the huma issue note that to reproduce it this was happening for embedded properties (booleans in our case, but the type might not be related) |
2 participants
Enhanced Go struct tags throughout
pkg/api/v0/types.goandpkg/model/types.goto better align with the OpenAPI specification.Changes
doctags with detailed field descriptions matching OpenAPIexampletags showing valid values for fields (e.g.,"io.github.user/weather","npx")formattags (uri,date-time) for better type specificationpatternandenumtags for validation constraintsplaceholderfield toInputstruct matching OpenAPI specThese changes improve code clarity and make the types more self-documenting, while maintaining compatibility with the OpenAPI schema definitions in
docs/reference/api/openapi.yaml.