docs: add option tables, fix 27 inaccuracies, add API sync CI test#343
docs: add option tables, fix 27 inaccuracies, add API sync CI test#343
Conversation
Add comprehensive option tables to builder/deep_dive.md (8 new tables: Tag, Server, Server Variable, Response Detail, Request Body Detail, Advanced Operation, Parameter Validation, Generic Naming) and walker/deep_dive.md (handler registration table, input options table). Remove 58 sourceException entries from the doctest now that all builder and walker With* functions are formally documented in option tables. Closes #342 Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Full audit found nonexistent functions (WithDocument, ValidateParsed, Build, ToYAML, ResponseConfig), wrong signatures (WithParameter, WithRequestBody, WithCollisionHandlerFor args reversed), internal package refs (severity.SeverityCritical), deprecated usage (WithMaxDepth), and pointer/value mismatches (WithParsed missing dereference). Files fixed: whitepaper, developer-guide, benchmarks, breaking-changes, README, example READMEs (quickstart, walker, workflows, programmatic-api), and builder/walker deep_dive review improvements. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
New TestDocCodeExampleAPISync test in internal/doctest/ scans all documentation markdown files (93 files) and verifies that Go code examples reference symbols that actually exist in the public packages. Catches: renamed/removed functions, nonexistent types/constants, internal package references (e.g., severity.SeverityCritical). Uses go/ast to build symbol tables for all 12 public packages. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
📝 WalkthroughWalkthroughThis PR updates documentation and examples to reflect numerous public API changes (option-based APIs, WithParsed usage, renamed symbols), adds comprehensive builder and walker option tables, and introduces a doctest that validates documentation code examples reference exported public symbols. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
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 |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #343 +/- ##
=======================================
Coverage 84.72% 84.72%
=======================================
Files 193 193
Lines 27273 27273
=======================================
Hits 23107 23107
Misses 2847 2847
Partials 1319 1319 🚀 New features to boost your workflow:
|
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 (3)
examples/walker/reference-collector/main.go (1)
3-8:⚠️ Potential issue | 🟡 MinorUpdate the header comment to the renamed option.
The bullet list still says “WithMaxDepth” while the code uses
WithMaxSchemaDepth. Please update the comment to avoid confusion.Suggested comment tweak
-// - Configure WithMaxDepth for schema traversal +// - Configure WithMaxSchemaDepth for schema traversalAlso applies to: 63-66
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@examples/walker/reference-collector/main.go` around lines 3 - 8, Update the header comment bullets to reference the renamed option WithMaxSchemaDepth instead of WithMaxDepth; change any occurrences in the top-of-file comment and the later comment block around lines referencing the schema traversal option (the comment that currently mentions WithMaxDepth) so they consistently mention WithMaxSchemaDepth to match the code that uses the option.examples/walker/security-audit/README.md (1)
78-86:⚠️ Potential issue | 🟡 Minor
currentPathTemplateis undefined in the snippet.Either show where it’s set, or use
wc.PathTemplatedirectly so the snippet is self-contained.Example tweak
- if len(op.Security) == 0 && !isInternalPath(currentPathTemplate) { + if len(op.Security) == 0 && !isInternalPath(wc.PathTemplate) {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@examples/walker/security-audit/README.md` around lines 78 - 86, The snippet references an undefined variable currentPathTemplate; update the walker.WithOperationHandler block to use the WalkContext path template (wc.PathTemplate) or explicitly show where currentPathTemplate is set so the example is self-contained — specifically change the condition that checks !isInternalPath(currentPathTemplate) to use !isInternalPath(wc.PathTemplate) (or add a preceding assignment for currentPathTemplate) so the logic around op.Security and isInternalPath is valid and locatable in the walker.WithOperationHandler handler.docs/developer-guide.md (1)
1314-1359:⚠️ Potential issue | 🟡 MinorAdd missing imports to the builder example code.
The example code uses
log.Fatal(),fmt.Println(), andyaml.Marshal()but the import block is missingfmt,log, and the YAML package. Add these imports so the example compiles as shown:Required import additions
import ( + "fmt" + "log" "net/http" + + "go.yaml.in/yaml/v4" "github.com/erraggy/oastools/builder" "github.com/erraggy/oastools/parser" )🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/developer-guide.md` around lines 1314 - 1359, The example is missing imports for packages used later (fmt, log, and the YAML package) which breaks compilation; update the import block that currently lists "net/http", "github.com/erraggy/oastools/builder", and "github.com/erraggy/oastools/parser" to also include fmt, log, and the YAML package you intend to use (e.g., "gopkg.in/yaml.v3" or "sigs.k8s.io/yaml") so calls like fmt.Println, log.Fatal, and yaml.Marshal in the code that builds the spec (builder.New, spec.BuildOAS3, yaml.Marshal(doc), fmt.Println(string(data)), log.Fatal(err)) resolve correctly.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@builder/deep_dive.md`:
- Around line 1266-1267: The builder currently restricts
WithParamExclusiveMinimum(bool) and WithParamExclusiveMaximum(bool) to boolean
flags which prevents emitting OAS 3.1+ numeric exclusive bounds; update these
builder methods to accept a numeric-or-bool type (e.g., interface{} or
float64|bool union pattern) and adjust the spec generation/serialization logic
that emits ExclusiveMinimum/ExclusiveMaximum so it outputs a number for OAS 3.1+
targets and a boolean for OAS 2.0/3.0; ensure input validation in
WithParamExclusiveMinimum/WithParamExclusiveMaximum enforces allowed types and
update any downstream code that consumes the builder to handle the new value
type, or alternatively add a clear note in the builder docs next to
WithParamExclusiveMinimum/WithParamExclusiveMaximum describing the current
limitation and supported OAS versions.
In `@examples/walker/reference-collector/README.md`:
- Around line 154-161: Update the README section header to reflect the new API
name: change the "MaxDepth Configuration" heading to something like "Max Schema
Depth" or "WithMaxSchemaDepth Configuration" so it matches the documented
function walker.WithMaxSchemaDepth used in the example (and any references to
walker.Walk(parseResult, walker.WithMaxSchemaDepth(50), ...)); ensure the header
wording clearly references WithMaxSchemaDepth to avoid mismatch between heading
and code.
In `@examples/workflows/README.md`:
- Around line 176-181: The example currently ignores the error from
parser.ParseWithOptions and then dereferences parsed (a *parser.ParseResult)
when calling fixer.FixWithOptions(fixer.WithParsed(*parsed)) and
validator.ValidateWithOptions(validator.WithParsed(*parsed)), which can panic if
parsing failed; update the snippet to check the returned error and the parsed
value before dereferencing (e.g., if err != nil or parsed == nil, log/return),
and only call FixWithOptions/ValidateWithOptions with fixer.WithParsed(*parsed)
and validator.WithParsed(*parsed) after confirming parsed is non-nil.
In `@README.md`:
- Around line 52-55: The snippet risks nil-pointer panics because
parser.ParseWithOptions may return a nil *parser.ParseResult; update the flow
around parser.ParseWithOptions (the result variable) to capture and check the
error and ensure result != nil before dereferencing or passing *result into
validator.ValidateWithOptions and fixer.FixWithOptions; if parsing fails, handle
or return the error (or log and exit) so validator.ValidateWithOptions and
fixer.FixWithOptions never receive a nil dereference of result.
---
Outside diff comments:
In `@docs/developer-guide.md`:
- Around line 1314-1359: The example is missing imports for packages used later
(fmt, log, and the YAML package) which breaks compilation; update the import
block that currently lists "net/http", "github.com/erraggy/oastools/builder",
and "github.com/erraggy/oastools/parser" to also include fmt, log, and the YAML
package you intend to use (e.g., "gopkg.in/yaml.v3" or "sigs.k8s.io/yaml") so
calls like fmt.Println, log.Fatal, and yaml.Marshal in the code that builds the
spec (builder.New, spec.BuildOAS3, yaml.Marshal(doc), fmt.Println(string(data)),
log.Fatal(err)) resolve correctly.
In `@examples/walker/reference-collector/main.go`:
- Around line 3-8: Update the header comment bullets to reference the renamed
option WithMaxSchemaDepth instead of WithMaxDepth; change any occurrences in the
top-of-file comment and the later comment block around lines referencing the
schema traversal option (the comment that currently mentions WithMaxDepth) so
they consistently mention WithMaxSchemaDepth to match the code that uses the
option.
In `@examples/walker/security-audit/README.md`:
- Around line 78-86: The snippet references an undefined variable
currentPathTemplate; update the walker.WithOperationHandler block to use the
WalkContext path template (wc.PathTemplate) or explicitly show where
currentPathTemplate is set so the example is self-contained — specifically
change the condition that checks !isInternalPath(currentPathTemplate) to use
!isInternalPath(wc.PathTemplate) (or add a preceding assignment for
currentPathTemplate) so the logic around op.Security and isInternalPath is valid
and locatable in the walker.WithOperationHandler handler.
Update the package comment and README heading to use the renamed WithMaxSchemaDepth, missed during the rename in the prior commit. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
examples/walker/reference-collector/README.md (1)
104-124:⚠️ Potential issue | 🟡 MinorDocumentation examples omit the empty-name guard present in
main.go.Both snippets apply
extractSchemaNamewithout checking whether the result is"". For$refstrings that don't follow the#/components/schemas/<name>pattern,extractSchemaNamereturns"", causing a silent empty-key entry in the map. The actual implementation inmain.go(lines 73 and 81–84) guards against this in both places.📝 Suggested fix for the Reference Tracking snippet
walker.WithSchemaHandler(func(wc *walker.WalkContext, schema *parser.Schema) walker.Action { if schema.Ref != "" { schemaName := extractSchemaName(schema.Ref) - refs[schemaName] = append(refs[schemaName], wc.JSONPath) + if schemaName != "" { + refs[schemaName] = append(refs[schemaName], wc.JSONPath) + } } return walker.Continue })📝 Suggested fix for the Array Items snippet
if items, ok := schema.Items.(map[string]any); ok { if ref, ok := items["$ref"].(string); ok { schemaName := extractSchemaName(ref) - collector.SchemaRefs[schemaName] = append(collector.SchemaRefs[schemaName], wc.JSONPath+".items") + if schemaName != "" { + collector.SchemaRefs[schemaName] = append(collector.SchemaRefs[schemaName], wc.JSONPath+".items") + } } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@examples/walker/reference-collector/README.md` around lines 104 - 124, The documentation examples call extractSchemaName and then append into refs or collector.SchemaRefs without checking for an empty result; update the walker.WithSchemaHandler snippet (where refs[schemaName] = append(..., wc.JSONPath)) and the Array Items snippet (where collector.SchemaRefs[schemaName] = append(..., wc.JSONPath+".items")) to first verify schemaName := extractSchemaName(...) is not empty (schemaName != "") before modifying the map, matching the empty-name guard used in main.go.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In `@examples/walker/reference-collector/README.md`:
- Around line 104-124: The documentation examples call extractSchemaName and
then append into refs or collector.SchemaRefs without checking for an empty
result; update the walker.WithSchemaHandler snippet (where refs[schemaName] =
append(..., wc.JSONPath)) and the Array Items snippet (where
collector.SchemaRefs[schemaName] = append(..., wc.JSONPath+".items")) to first
verify schemaName := extractSchemaName(...) is not empty (schemaName != "")
before modifying the map, matching the empty-name guard used in main.go.
---
Duplicate comments:
In `@examples/walker/reference-collector/README.md`:
- Around line 154-163: The README section "MaxSchemaDepth Configuration" and its
example call walker.WithMaxSchemaDepth(50) are correct and match the
walker.WithMaxSchemaDepth option defined in walker/options.go, so no code
changes are required; leave the heading and example usage as-is.
Addressing outside-diff review comments
|
Summary
TestDocCodeExampleAPISyncCI test that scans 93 markdown files and verifies everypackage.Symbolreference in Go code examples against AST-extracted symbol tables for all 12 public packagesCloses #342
Details
Option Tables (commit 1)
Inaccuracies Fixed (commit 2)
WithDocument,ValidateParsed,Build,ToYAML,ResponseConfigWithParameterpositional args,WithRequestBodyarg order,WithCollisionHandlerForreversedseverity.SeverityCritical→differ.SeverityCriticalWithMaxDepth→WithMaxSchemaDepthWithParsed(result)→WithParsed(*result)joiner.WriteResult()→j.WriteResult()CI Verification Test (commit 3)
TestDocCodeExampleAPISyncusesgo/astto build symbol tables for all 12 public packagespkg.Symbol) exists as an exported symbolseverity.*,httputil.*)Test plan
make checkpasses (8498 tests)TestDocCodeExampleAPISyncpasses with 93 subtestsTestDeepDiveOptionTablespasses (58 fewer exceptions)WithNonexistentin README.md, test caught both occurrences with file:line and clear message🤖 Generated with Claude Code