docs: fix ~100 inaccuracies and add CI prevention checks#341
Merged
Conversation
|
Warning Rate limit exceeded
⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📝 WalkthroughWalkthroughAdds Lychee link-checking to docs CI and Makefile targets, introduces structured per-subcommand flag types and Setup*Flags for walk commands, adds CLI/docs consistency tests and an exported-options doctest, and updates numerous deep-dive docs reflecting public API renames and additions across packages. Changes
Sequence Diagram(s)mermaid Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
🚥 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)
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 #341 +/- ##
=======================================
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:
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 8
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (5)
docs/cli-reference.md (1)
1846-1851:⚠️ Potential issue | 🟡 MinorSecurity Considerations section is now inaccurate — update to reflect opt-in HTTP ref resolution.
Lines 375-376 of this same file add
--resolve-http-refsand--insecureto theparsecommand, but the Security Considerations section still states that "HTTP(S) references are not supported for security reasons." The statement is now factually wrong; HTTP$refresolution is supported opt-in.📝 Suggested correction
-1. **External References**: Only local file references are supported for `$ref` values. HTTP(S) references are not supported for security reasons. +1. **External References**: Local file references are supported by default. HTTP(S) `$ref` resolution is opt-in via `--resolve-http-refs` in the `parse` command; use `--insecure` to disable TLS verification (not recommended for production use).🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/cli-reference.md` around lines 1846 - 1851, Update the "Security Considerations" section to reflect that HTTP(S) $ref resolution is supported only when explicitly enabled via the parse command flags --resolve-http-refs (and that --insecure allows skipping TLS verification), i.e., change the statement that "HTTP(S) references are not supported" to say HTTP(S) refs are supported opt-in via --resolve-http-refs, warn that enabling it can introduce remote content/trust risks and recommend using --insecure only when necessary, and keep the existing note that local file refs remain restricted to the base directory to prevent path traversal; reference the flags --resolve-http-refs and --insecure and the "Security Considerations" heading when making the edits.generator/deep_dive.md (1)
1357-1379:⚠️ Potential issue | 🟡 MinorAdd missing options to Available Options table:
WithSplitByPathPrefix,WithMaxTypesPerFile, andWithMaxOperationsPerFile.These three functions exist as public options in
generator/generator.go(lines 552–586), appear in the Generator struct fields, and are documented indoc.go, but are absent from the Available Options table at lines 1357–1379.📝 Suggested additions after `WithSplitByTag(bool)`
| `WithSplitByTag(bool)` | Group operations by tag | +| `WithSplitByPathPrefix(bool)` | Group operations by path prefix (fallback) | +| `WithMaxTypesPerFile(int)` | Max types per generated file | +| `WithMaxOperationsPerFile(int)` | Max operations per generated file |🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@generator/deep_dive.md` around lines 1357 - 1379, The Available Options table in deep_dive.md is missing three public option functions—WithSplitByPathPrefix, WithMaxTypesPerFile, and WithMaxOperationsPerFile—that exist in generator/generator.go and as fields on Generator; update the table (near the WithSplitByTag row) to add entries for these three options with short descriptions (e.g., WithSplitByPathPrefix(bool) | Split files by path prefix, WithMaxTypesPerFile(int) | Max types per file, WithMaxOperationsPerFile(int) | Max operations per file) so the docs reflect the public API declared by the WithSplitByPathPrefix/WithMaxTypesPerFile/WithMaxOperationsPerFile functions and Generator struct fields.httpvalidator/deep_dive.md (1)
951-956:⚠️ Potential issue | 🟡 MinorAlign the ValidatedParams example with map[string]any.
The new public result types use
map[string]any; the example still showsmap[string]interface{}. Keeping examples consistent will reduce confusion.✏️ Suggested update
type ValidatedParams struct { - PathParams map[string]interface{} - QueryParams map[string]interface{} - HeaderParams map[string]interface{} - CookieParams map[string]interface{} + PathParams map[string]any + QueryParams map[string]any + HeaderParams map[string]any + CookieParams map[string]any }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@httpvalidator/deep_dive.md` around lines 951 - 956, Update the ValidatedParams struct to use the modern alias type by replacing all occurrences of map[string]interface{} with map[string]any in the ValidatedParams definition so it matches the new public result types; specifically edit the ValidatedParams struct declaration (ValidatedParams, PathParams, QueryParams, HeaderParams, CookieParams) to use map[string]any.converter/deep_dive.md (1)
990-993:⚠️ Potential issue | 🟡 MinorClarify the 3.2.0 recommendation vs supported conversions list.
The “latest features” bullet recommends
3.2.0, but the supported conversions section only lists 3.0.x ↔ 3.1.x. Either add 3.2.x support to the earlier list or narrow this recommendation to the supported range.✏️ Suggested clarification
- - For latest features: `3.1.0` or `3.2.0` + - For latest features (within converter support): `3.1.0`🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@converter/deep_dive.md` around lines 990 - 993, Update the discrepancy between the “Use appropriate target versions” bullet (which recommends `3.2.0`) and the supported conversions list that only documents 3.0.x ↔ 3.1.x: either add explicit support for 3.2.x to the supported conversions documentation (and update any conversion matrices or examples) or change the “latest features” recommendation in the "8. **Use appropriate target versions**" paragraph to only list versions documented as supported (e.g., `3.1.0` or `3.0.3`); ensure you edit the "Use appropriate target versions" section and the "supported conversions" section so both mention the same set of supported target versions.docs/whitepaper.md (1)
2043-2046:⚠️ Potential issue | 🟠 MajorFix the Go install command to match the documented install path.
The source install command in this section doesn’t match the repository’s stated install path and will fail for users.
✏️ Suggested correction
-go install github.com/erraggy/oastools/cmd/oastools@latest +go install github.com/erraggy/oastools/cmd/oastoolslatest@latestBased on learnings: Install oastools via Homebrew using
brew install erraggy/oastools/oastoolsor from source usinggo install github.com/erraggy/oastools/cmd/oastoolslatest(requires Go 1.24+).🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/whitepaper.md` around lines 2043 - 2046, Replace the incorrect source install line that uses the wrong module path with the documented options: recommend the Homebrew install `brew install erraggy/oastools/oastools` or, for building from source, update the Go install line to the correct module path `go install github.com/erraggy/oastools/cmd/oastoolslatest` and note it requires Go 1.24+; ensure the README/whitepaper entry mentioning the install command is updated accordingly.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.github/workflows/docs.yml:
- Around line 39-43: Replace the floating action reference
"lycheeverse/lychee-action@v2" in the "Check links" job with the full commit SHA
for the specific v2.7.0 commit (i.e., pin the uses field to
lycheeverse/lychee-action@<FULL_COMMIT_SHA>) so the job runs a fixed version;
then restrict permissions for that job to "contents: read" (or split the
link-check into its own job with contents: read) while keeping the deploy job
that needs elevated access with "contents: write" so the link-check step does
not run with broad write permissions.
In `@cmd/oastools/commands/walk_parameters.go`:
- Line 71: The comparison between info.Method and flags.Method is case-sensitive
while flags.Method is lowercased at flags.Method =
strings.ToLower(flags.Method); update the filter to do a case-insensitive match
or normalize custom operation keys: either use case-insensitive comparison
(e.g., strings.EqualFold(info.Method, flags.Method) when checking in the walker)
or ensure AdditionalOperations keys are normalized to lowercase when parsed so
that info.Method and flags.Method are compared consistently; update the code
paths referencing flags.Method and info.Method (and where AdditionalOperations
keys are created) accordingly.
In `@differ/deep_dive.md`:
- Line 56: The example that builds the categoryOrder slice omits the new
CategoryExtension constant so extension (x-*) changes get dropped; update the
hardcoded categoryOrder used in the "Grouping Changes by Category" example to
include CategoryExtension alongside the other categories (ensure any logic that
iterates or maps over categoryOrder still handles extension entries), and verify
that the example’s grouping code (the categoryOrder slice and the grouping loop
that consumes it) will preserve and display CategoryExtension entries instead of
discarding them.
In `@lychee.toml`:
- Around line 4-5: The lychee.toml enables cache via cache = true (writing
.lycheecache) but CI workflows don't persist it; update your GitHub Actions
workflow that runs lychee (the job that uses lychee/action or runs lychee CLI)
to add an actions/cache step that restores/saves the .lycheecache file between
runs (use a cache key based on runner OS and lychee.toml or commit hash), so the
.lycheecache created by cache = true is restored at the start and saved at the
end; follow the lychee-action docs for recommended cache key/paths and place the
cache step before the lychee run.
- Line 1: Update the header comment string in the lychee.toml file: replace the
incorrect leading-dot comment "# .lychee.toml — link checker configuration" with
"# lychee.toml — link checker configuration" so the comment matches the actual
filename and lychee's auto-discovery; edit the top comment line in lychee.toml
accordingly.
In `@Makefile`:
- Around line 575-588: The docs-check target currently lists docs-prepare and
lint-links as prerequisites but docs-prepare is redundant because lint-links
already depends on docs-prepare; update the Makefile so the docs-check rule uses
only lint-links as its prerequisite (keep the .PHONY declarations unchanged) to
remove the duplicate dependency and avoid confusion (modify the docs-check
target definition that currently reads "docs-check: docs-prepare lint-links" to
"docs-check: lint-links").
In `@overlay/deep_dive.md`:
- Around line 235-246: Remove the ToParseResult() entry from the ApplyResult
field table and place it in a methods section (or delete it) because
ToParseResult() is a method, not a struct field; update the ApplyResult
documentation table to list only actual fields (Document, SourceFormat,
ActionsApplied, ActionsSkipped, Changes, Warnings, StructuredWarnings /
ApplyWarnings) and add a separate "Methods" subsection documenting
ToParseResult() with its signature and brief description.
In `@parser/deep_dive.md`:
- Line 309: The variable name "copy" shadows the Go predeclared identifier;
rename the variable assigned from doc.DeepCopy() (currently "copy :=
doc.DeepCopy()") to a descriptive, non-predeclared name such as "docCopy" or
"copiedDoc", and update all subsequent uses in the example to the new name so
the builtin copy function is not masked and go vet warnings are avoided.
---
Outside diff comments:
In `@converter/deep_dive.md`:
- Around line 990-993: Update the discrepancy between the “Use appropriate
target versions” bullet (which recommends `3.2.0`) and the supported conversions
list that only documents 3.0.x ↔ 3.1.x: either add explicit support for 3.2.x to
the supported conversions documentation (and update any conversion matrices or
examples) or change the “latest features” recommendation in the "8. **Use
appropriate target versions**" paragraph to only list versions documented as
supported (e.g., `3.1.0` or `3.0.3`); ensure you edit the "Use appropriate
target versions" section and the "supported conversions" section so both mention
the same set of supported target versions.
In `@docs/cli-reference.md`:
- Around line 1846-1851: Update the "Security Considerations" section to reflect
that HTTP(S) $ref resolution is supported only when explicitly enabled via the
parse command flags --resolve-http-refs (and that --insecure allows skipping TLS
verification), i.e., change the statement that "HTTP(S) references are not
supported" to say HTTP(S) refs are supported opt-in via --resolve-http-refs,
warn that enabling it can introduce remote content/trust risks and recommend
using --insecure only when necessary, and keep the existing note that local file
refs remain restricted to the base directory to prevent path traversal;
reference the flags --resolve-http-refs and --insecure and the "Security
Considerations" heading when making the edits.
In `@docs/whitepaper.md`:
- Around line 2043-2046: Replace the incorrect source install line that uses the
wrong module path with the documented options: recommend the Homebrew install
`brew install erraggy/oastools/oastools` or, for building from source, update
the Go install line to the correct module path `go install
github.com/erraggy/oastools/cmd/oastoolslatest` and note it requires Go 1.24+;
ensure the README/whitepaper entry mentioning the install command is updated
accordingly.
In `@generator/deep_dive.md`:
- Around line 1357-1379: The Available Options table in deep_dive.md is missing
three public option functions—WithSplitByPathPrefix, WithMaxTypesPerFile, and
WithMaxOperationsPerFile—that exist in generator/generator.go and as fields on
Generator; update the table (near the WithSplitByTag row) to add entries for
these three options with short descriptions (e.g., WithSplitByPathPrefix(bool) |
Split files by path prefix, WithMaxTypesPerFile(int) | Max types per file,
WithMaxOperationsPerFile(int) | Max operations per file) so the docs reflect the
public API declared by the
WithSplitByPathPrefix/WithMaxTypesPerFile/WithMaxOperationsPerFile functions and
Generator struct fields.
In `@httpvalidator/deep_dive.md`:
- Around line 951-956: Update the ValidatedParams struct to use the modern alias
type by replacing all occurrences of map[string]interface{} with map[string]any
in the ValidatedParams definition so it matches the new public result types;
specifically edit the ValidatedParams struct declaration (ValidatedParams,
PathParams, QueryParams, HeaderParams, CookieParams) to use map[string]any.
erraggy
force-pushed
the
docs/accuracy-audit-ci-checks
branch
from
81458eb to
bd74158
Compare
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
cmd/oastools/commands/walk_security.go (1)
151-160:⚠️ Potential issue | 🟠 MajorDetail output should serialize the raw security scheme.
Wrapping the scheme with name metadata breaks the raw OAS fragment contract for
--detail. Renderinfo.SecuritySchemedirectly.🔧 Suggested fix
- for _, info := range schemes { - view := securityDetailView{ - Name: info.Name, - SecurityScheme: info.SecurityScheme, - } - if err := RenderDetail(os.Stdout, view, flags.Format); err != nil { - return fmt.Errorf("walk security: rendering detail: %w", err) - } - } + for _, info := range schemes { + if err := RenderDetail(os.Stdout, info.SecurityScheme, flags.Format); err != nil { + return fmt.Errorf("walk security: rendering detail: %w", err) + } + }Based on learnings: In the oastools walk command, the detail mode (--detail) should render only the raw OAS node without wrapper metadata.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@cmd/oastools/commands/walk_security.go` around lines 151 - 160, The detail output currently wraps the security scheme in securityDetailView inside renderSecurityDetail; change it to pass the raw OAS node directly by calling RenderDetail with info.SecurityScheme (not the wrapper) so --detail serializes the original security scheme fragment; update any variable usage in renderSecurityDetail to remove the wrapper and call RenderDetail(os.Stdout, info.SecurityScheme, flags.Format) and ensure error handling remains the same.cmd/oastools/commands/walk_schemas.go (1)
123-134:⚠️ Potential issue | 🟠 MajorDetail output should serialize the raw schema, not a wrapper.
--detailcurrently emitsschemaDetailViewmetadata, which prevents the output from being a valid OAS schema fragment. Renderinfo.Schemadirectly (and drop the wrapper if unused).🔧 Suggested fix
- for _, info := range filtered { - view := schemaDetailView{ - Name: info.Name, - JSONPath: info.JSONPath, - IsComponent: info.IsComponent, - Schema: info.Schema, - } - if err := RenderDetail(os.Stdout, view, flags.Format); err != nil { - return fmt.Errorf("walk schemas: rendering detail: %w", err) - } - } + for _, info := range filtered { + if err := RenderDetail(os.Stdout, info.Schema, flags.Format); err != nil { + return fmt.Errorf("walk schemas: rendering detail: %w", err) + } + }Based on learnings: In the oastools walk command, the detail mode (--detail) should render only the raw OAS node without wrapper metadata.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@cmd/oastools/commands/walk_schemas.go` around lines 123 - 134, The detail path currently wraps the OAS node in schemaDetailView; change it to render the raw schema node by passing info.Schema directly to RenderDetail (i.e., remove creation/usage of schemaDetailView in the flags.Detail branch and call RenderDetail(os.Stdout, info.Schema, flags.Format)), and then remove the now-unused schemaDetailView type or any related fields/usages; ensure error handling remains (wrap any RenderDetail error as before) and update any function signature or imports if RenderDetail's parameter types require adjustment.fixer/deep_dive.md (1)
230-239:⚠️ Potential issue | 🟡 MinorRemove
ToParseResult()from the FixResult field table.It’s a method, not a field, so it should be moved to a methods subsection or referenced in the Package Chaining section instead of the fields table.
📝 Suggested doc tweak
| `SourceFormat` | `SourceFormat` | Preserved format | -| `ToParseResult()` | `*parser.ParseResult` | Converts result for package chaining | + +#### Methods +`ToParseResult() *parser.ParseResult` — Converts result for package chaining.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@fixer/deep_dive.md` around lines 230 - 239, The FixResult documentation currently lists ToParseResult() as a field; remove the `ToParseResult()` row from the "FixResult Fields" table and instead document it under a methods subsection (e.g., "FixResult Methods") or reference it in the "Package Chaining" section; update the FixResult entry to only include true fields (`Document`, `Fixes`, `FixCount`, `SourceFormat`) and add a short method signature/description for `ToParseResult()` in the new methods subsection or the package-chaining discussion so readers can find it as a method (reference symbol: FixResult and method name ToParseResult()).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@docs/cli-reference.md`:
- Around line 375-377: Update the Security Considerations note to reflect that
HTTP/HTTPS $ref resolution is now supported when explicitly enabled via the
--resolve-http-refs flag and that certificate verification can be bypassed with
--insecure; replace the incorrect statement that HTTP(S) refs are not supported
with guidance that these flags must be used intentionally, describe the risks of
enabling --resolve-http-refs and the additional risk of disabling TLS
verification with --insecure, and mention that --resolve-refs is required as a
prerequisite to --resolve-http-refs so readers understand the opt-in nature of
this behavior.
In `@internal/doctest/doc_test.go`:
- Around line 3-270: The test TestDeepDiveOptionTables and its helper calls
(extractWithFunctions, extractDocOptions) use raw testing.T methods; switch to
stretchr/testify by adding imports for "github.com/stretchr/testify/require" and
"github.com/stretchr/testify/assert" and replace t.Fatal/t.Fatalf with
require.NoError/require.FailNow where appropriate, replace t.Skipf with
require.NotEmpty/require.False or keep t.Skipf but prefer
require.Len/require.NotEmpty for extracted slices, and replace t.Errorf checks
for membership/non-existence with assert.Contains/assert.NotContains or
require.Contains/require.NotContains depending on whether you want to stop the
test immediately; also update the checks that sourceExceptions/docExceptions
entries are not stale to use assert/require so the test matches the project's
assertion style while still referencing the same variables (sourceExceptions,
docExceptions) and helper functions (extractWithFunctions, extractDocOptions).
---
Outside diff comments:
In `@cmd/oastools/commands/walk_schemas.go`:
- Around line 123-134: The detail path currently wraps the OAS node in
schemaDetailView; change it to render the raw schema node by passing info.Schema
directly to RenderDetail (i.e., remove creation/usage of schemaDetailView in the
flags.Detail branch and call RenderDetail(os.Stdout, info.Schema,
flags.Format)), and then remove the now-unused schemaDetailView type or any
related fields/usages; ensure error handling remains (wrap any RenderDetail
error as before) and update any function signature or imports if RenderDetail's
parameter types require adjustment.
In `@cmd/oastools/commands/walk_security.go`:
- Around line 151-160: The detail output currently wraps the security scheme in
securityDetailView inside renderSecurityDetail; change it to pass the raw OAS
node directly by calling RenderDetail with info.SecurityScheme (not the wrapper)
so --detail serializes the original security scheme fragment; update any
variable usage in renderSecurityDetail to remove the wrapper and call
RenderDetail(os.Stdout, info.SecurityScheme, flags.Format) and ensure error
handling remains the same.
In `@fixer/deep_dive.md`:
- Around line 230-239: The FixResult documentation currently lists
ToParseResult() as a field; remove the `ToParseResult()` row from the "FixResult
Fields" table and instead document it under a methods subsection (e.g.,
"FixResult Methods") or reference it in the "Package Chaining" section; update
the FixResult entry to only include true fields (`Document`, `Fixes`,
`FixCount`, `SourceFormat`) and add a short method signature/description for
`ToParseResult()` in the new methods subsection or the package-chaining
discussion so readers can find it as a method (reference symbol: FixResult and
method name ToParseResult()).
---
Duplicate comments:
In @.github/workflows/docs.yml:
- Around line 39-43: The workflow uses the mutable action reference
lycheeverse/lychee-action@v2 in the "Check links" step and runs with
workflow-level contents: write — change the action reference to a pinned commit
SHA for the intended v2 release (e.g., replace lycheeverse/lychee-action@v2 with
lycheeverse/lychee-action@<full-commit-sha> for the chosen v2.7.0 release) and
add a minimal permissions block for the link-check job setting permissions:
contents: read to follow least privilege; update the job that contains the
"Check links" step to include that permissions block so the action executes with
read-only repo access.
In `@cmd/oastools/commands/walk_parameters.go`:
- Line 71: The code lowercases flags.Method at assignment but later compares it
to info.Method with strict equality, causing mismatches for case-sensitive
AdditionalOperations keys; update the comparison to be case-insensitive (for
example use strings.EqualFold(info.Method, flags.Method)) or stop normalizing
flags.Method and instead normalize both sides consistently so that the check in
the function that compares info.Method to flags.Method (the equality at the
location referencing info.Method != flags.Method) reliably matches OAS 3.2+
AdditionalOperations keys.
In `@differ/deep_dive.md`:
- Line 56: Add the missing CategoryExtension entry to both the reference table
and the grouping example: update the reference table row for `CategoryExtension`
(the table listing specification extensions) and ensure the example
object/section that demonstrates grouping includes the `CategoryExtension` field
so they are consistent; search for "CategoryExtension", "Specification extension
(x-*) changes", and the grouping example to locate and modify the corresponding
table row and example block.
In `@parser/deep_dive.md`:
- Around line 305-313: The DeepCopy example is now correct: use the typed
document from result.OAS3Document(), create a deep copy into docCopy, and mutate
docCopy (e.g., docCopy.Info.Title = "Modified API") while leaving doc unchanged;
no code changes required—ensure the variables result, doc, and docCopy remain
used exactly as shown.
bd74158 to
ce1d40b
Compare
There was a problem hiding this comment.
Actionable comments posted: 5
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
generator/deep_dive.md (1)
1390-1412:⚠️ Potential issue | 🟡 MinorTwo fields are grouped under incorrect section comments.
PackageName string(line 1394) is under// Version info— package name is output configuration, not source version info. A more accurate heading could be// Source and output infoorPackageNameshould sit in its own section above the version fields.
Success bool(line 1405) falls under// Timing and sizebecause the PR added the blank line (1406) as the section terminator afterStats.Successdescribes the generation outcome and belongs closer to// Issues encountered, not in the timing/size group.📝 Suggested reorganization
- // Version info - SourceVersion string - SourceOASVersion parser.OASVersion - SourceFormat parser.SourceFormat - PackageName string + // Package name for the generated output + PackageName string + + // Source version info + SourceVersion string + SourceOASVersion parser.OASVersion + SourceFormat parser.SourceFormat // Statistics GeneratedTypes int GeneratedOperations int // Timing and size LoadTime time.Duration GenerateTime time.Duration SourceSize int64 Stats parser.DocumentStats - Success bool + Success bool + // Issues encountered Issues []GenerateIssue🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@generator/deep_dive.md` around lines 1390 - 1412, The struct comment grouping is wrong: move the PackageName field out from under the "// Version info" block into a more accurate section (either rename "// Version info" to "// Source and output info" or place PackageName alongside other output/config fields) and move the Success bool out of the "// Timing and size" block into the "Issues encountered" area (or place it directly after Issues/InfoCount/WarningCount/CriticalCount) so each field (PackageName, Success) sits under the appropriate comment; update or adjust the surrounding comment lines to preserve logical grouping and spacing for the struct containing SourceVersion, SourceOASVersion, PackageName, LoadTime, GenerateTime, SourceSize, Stats, Success, and Issues.converter/deep_dive.md (1)
131-165:⚠️ Potential issue | 🟡 MinorMissing
encoding/jsonimport in the complete example, and JSON output written to a YAML-named file.Two issues in this
package mainexample:
Missing import (compile error): The import block on lines 134–139 omits
"encoding/json", butjson.MarshalIndentis called on line 163. Anyone running this example will get anundefined: jsoncompiler error.Format mismatch:
json.MarshalIndentalways produces JSON. The input isswagger.yaml(YAML format), and the output is written toopenapi.yaml—but the file will contain JSON, not YAML.ConversionResult.SourceFormatis available to determine the original format; the example should either marshal in the correct format or useresult.ToParseResult()and the appropriate serializer.📝 Proposed fix for lines 131–165
import ( "fmt" + "encoding/json" "log" "os" "github.com/erraggy/oastools/converter" )And for the output, clarify format preservation:
- // Write the result using the converted document - data, _ := json.MarshalIndent(result.Document, "", " ") - os.WriteFile("openapi.yaml", data, 0644) + // Write the result - use the source format to choose the correct serializer + // result.SourceFormat == converter.SourceFormatYAML → use yaml.Marshal + // result.SourceFormat == converter.SourceFormatJSON → use json.MarshalIndent + data, _ := json.MarshalIndent(result.Document, "", " ") // adjust for YAML if needed + os.WriteFile("openapi.json", data, 0644)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@converter/deep_dive.md` around lines 131 - 165, The example is missing the encoding/json import and writes JSON to a .yaml file; add the "encoding/json" import (or a YAML serializer import) so json.MarshalIndent calls compile, and change the output logic to respect ConversionResult.SourceFormat by either: (a) using json.MarshalIndent(result.Document, ...) when result.SourceFormat indicates JSON and write to "openapi.json", or (b) call result.ToParseResult() and use the appropriate YAML serializer when SourceFormat is YAML and write to "openapi.yaml"; locate the conversion call (converter.ConvertWithOptions), result.Document and json.MarshalIndent in the snippet and update imports and output filename/serializer accordingly.cmd/oastools/commands/walk_schemas.go (1)
124-168: 🧹 Nitpick | 🔵 TrivialRendering done inline rather than via extracted helpers — minor inconsistency with sibling subcommands.
walk_operations.go,walk_security.go, andwalk_paths.goall extractrenderXDetail(items, flags WalkFlags)andrenderXSummary(items, flags WalkFlags)helpers that accept theWalkFlagsslice.walk_schemas.goinlines both paths inhandleWalkSchemas. There's no bug, but extractingrenderSchemasDetailandrenderSchemasSummarywould keep the package pattern uniform and make unit testing of the rendering logic easier.♻️ Proposed refactor
- // 3. Render: summary table or detail output - if flags.Detail { - for _, info := range filtered { - view := schemaDetailView{...} - if err := RenderDetail(os.Stdout, view, flags.Format); err != nil { - return fmt.Errorf("walk schemas: rendering detail: %w", err) - } - } - return nil - } - - headers := []string{...} - rows := make([][]string, 0, len(filtered)) - ... - if flags.Format != FormatText { - return RenderSummaryStructured(os.Stdout, headers, rows, flags.Format) - } - RenderSummaryTable(os.Stdout, headers, rows, flags.Quiet) - return nil + // 3. Render: summary table or detail output + if flags.Detail { + return renderSchemasDetail(filtered, flags.WalkFlags) + } + return renderSchemasSummary(filtered, flags.WalkFlags) } + +func renderSchemasDetail(schemas []*walker.SchemaInfo, flags WalkFlags) error { + for _, info := range schemas { + view := schemaDetailView{...} + if err := RenderDetail(os.Stdout, view, flags.Format); err != nil { + return fmt.Errorf("walk schemas: rendering detail: %w", err) + } + } + return nil +} + +func renderSchemasSummary(schemas []*walker.SchemaInfo, flags WalkFlags) error { + headers := []string{"NAME", "TYPE", "PROPERTIES", "LOCATION", "EXTENSIONS"} + rows := make([][]string, 0, len(schemas)) + for _, info := range schemas { ... } + if flags.Format != FormatText { + return RenderSummaryStructured(os.Stdout, headers, rows, flags.Format) + } + RenderSummaryTable(os.Stdout, headers, rows, flags.Quiet) + return nil +}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@cmd/oastools/commands/walk_schemas.go` around lines 124 - 168, handleWalkSchemas currently inlines both detailed and summary rendering; extract that logic into two helpers renderSchemasDetail(filtered []schemaInfo, flags WalkFlags) and renderSchemasSummary(filtered []schemaInfo, flags WalkFlags) and call them from handleWalkSchemas. Move the detail-loop that builds schemaDetailView and calls RenderDetail into renderSchemasDetail (returning error), and move the summary logic that builds headers/rows, uses FormatText, RenderSummaryStructured and RenderSummaryTable into renderSchemasSummary; preserve use of schemaDetailView, FormatText, RenderDetail, RenderSummaryStructured, RenderSummaryTable and return any errors up to handleWalkSchemas. Ensure signatures accept the same WalkFlags type and the filtered slice (the variable named filtered in the diff) so existing tests and callers need only a small change to call the new helpers.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@cmd/oastools/commands/cli_doc_test.go`:
- Around line 59-83: Replace the intermediate slice by populating a map directly
inside fs.VisitAll: instead of appending flag names to the registered slice and
later converting it to registeredSet, create registeredSet :=
make(map[string]bool) before calling fs.VisitAll and set registeredSet[f.Name] =
true (skipping single-char aliases and "help") inside the VisitAll callback;
then use registeredSet for both existence checks (the loop that ensures every
registered flag is documented and the loop that ensures every documented flag is
registered), keeping identifiers documented, cmdName and the t.Errorf call
unchanged.
- Around line 148-150: The code uses strings.TrimLeft(trimmed, "# ") which
removes any leading '#' or space characters instead of the literal "# " prefix;
change this to strip only the exact markdown header prefix (e.g., use
strings.TrimPrefix to remove "# " if present, or check strings.HasPrefix before
slicing) so headerText is computed deterministically; update the lines that
assign headerText (currently using trimmed → headerText) and keep the subsequent
TrimSpace and ToLower logic (headerText, headerLower) intact.
In `@converter/deep_dive.md`:
- Line 984: The guidance about marshaling result.Document lacks specificity;
update the text to reference ConversionResult.SourceFormat and instruct readers
to choose the marshaler accordingly (use a YAML marshaler when
ConversionResult.SourceFormat == "yaml" or equivalent, otherwise use a JSON
marshaler), and mention using result.ToParseResult() as an alternative for
chaining; ensure the documentation around result.Document and
ConversionResult.SourceFormat clearly states this conditional marshaler choice
so consumers know which encoder to use.
In `@fixer/deep_dive.md`:
- Around line 47-48: Update the explanatory text for disabled-by-default fixes
to distinguish performance-based disables from behavior/safety-based disables:
keep the performance rationale for FixTypeRenamedGenericSchema,
FixTypePrunedUnusedSchema, and FixTypePrunedEmptyPath, but explicitly state that
FixTypeDuplicateOperationId is disabled because it mutates operationId values (a
potentially breaking, opt-in change) and FixTypeStubMissingRef is disabled
because it injects synthetic placeholder content (unsafe to add silently in
production); reference the symbols FixTypeDuplicateOperationId,
FixTypeStubMissingRef, FixTypeRenamedGenericSchema, FixTypePrunedUnusedSchema,
and FixTypePrunedEmptyPath when editing the prose around lines 50–52.
- Around line 210-214: Update the documentation table so the option function
signatures match the actual code: change WithUserAgent(string) to
WithUserAgent(userAgent string), change WithOperationIdNamingConfig(config) to
WithOperationIdNamingConfig(config OperationIdNamingConfig), and make
WithSourceMap(sm *parser.SourceMap) use a semantic parameter name consistent
with other options (e.g., WithSourceMap(sourceMap *parser.SourceMap)) or match
whatever the actual symbol is in the code; ensure the table entries exactly
mirror the real signatures for WithUserAgent, WithOperationIdNamingConfig,
WithSourceMap, WithStubConfig, and WithStubResponseDescription.
---
Outside diff comments:
In `@cmd/oastools/commands/walk_schemas.go`:
- Around line 124-168: handleWalkSchemas currently inlines both detailed and
summary rendering; extract that logic into two helpers
renderSchemasDetail(filtered []schemaInfo, flags WalkFlags) and
renderSchemasSummary(filtered []schemaInfo, flags WalkFlags) and call them from
handleWalkSchemas. Move the detail-loop that builds schemaDetailView and calls
RenderDetail into renderSchemasDetail (returning error), and move the summary
logic that builds headers/rows, uses FormatText, RenderSummaryStructured and
RenderSummaryTable into renderSchemasSummary; preserve use of schemaDetailView,
FormatText, RenderDetail, RenderSummaryStructured, RenderSummaryTable and return
any errors up to handleWalkSchemas. Ensure signatures accept the same WalkFlags
type and the filtered slice (the variable named filtered in the diff) so
existing tests and callers need only a small change to call the new helpers.
In `@converter/deep_dive.md`:
- Around line 131-165: The example is missing the encoding/json import and
writes JSON to a .yaml file; add the "encoding/json" import (or a YAML
serializer import) so json.MarshalIndent calls compile, and change the output
logic to respect ConversionResult.SourceFormat by either: (a) using
json.MarshalIndent(result.Document, ...) when result.SourceFormat indicates JSON
and write to "openapi.json", or (b) call result.ToParseResult() and use the
appropriate YAML serializer when SourceFormat is YAML and write to
"openapi.yaml"; locate the conversion call (converter.ConvertWithOptions),
result.Document and json.MarshalIndent in the snippet and update imports and
output filename/serializer accordingly.
In `@generator/deep_dive.md`:
- Around line 1390-1412: The struct comment grouping is wrong: move the
PackageName field out from under the "// Version info" block into a more
accurate section (either rename "// Version info" to "// Source and output info"
or place PackageName alongside other output/config fields) and move the Success
bool out of the "// Timing and size" block into the "Issues encountered" area
(or place it directly after Issues/InfoCount/WarningCount/CriticalCount) so each
field (PackageName, Success) sits under the appropriate comment; update or
adjust the surrounding comment lines to preserve logical grouping and spacing
for the struct containing SourceVersion, SourceOASVersion, PackageName,
LoadTime, GenerateTime, SourceSize, Stats, Success, and Issues.
---
Duplicate comments:
In @.github/workflows/docs.yml:
- Around line 39-50: Update the "Check links" step to avoid the floating tag and
over-broad permissions: pin lycheeverse/lychee-action to commit
a8c4c7cb88f0c7386610c35eb25108e448569cb0 instead of `@v2`, and add a minimal
permissions block with contents: read for that step (or for the workflow/job) so
the link-checker runs with least privilege; locate the step by the step name
"Check links" and the uses entry "lycheeverse/lychee-action@v2" and replace the
tag and adjust the permissions accordingly.
In `@differ/deep_dive.md`:
- Line 56: Ensure the documentation consistently shows CategoryExtension in both
the category table and the categoryOrder example: verify the `CategoryExtension`
entry exists in the category table and that the `categoryOrder` example
references the same `CategoryExtension` name/definition, then remove the
duplicated review comment markers (`[duplicate_comment]`) so the doc no longer
contains redundant approval comments; check the `CategoryExtension` identifier
is identical in both places and update either the table or the example to match
if they differ.
In `@docs/cli-reference.md`:
- Around line 374-377: The Security Considerations section still claims HTTP(S)
$ref resolution is unsupported but the CLI now includes opt-in flags
--resolve-refs and --resolve-http-refs and the --insecure flag; update that
section to state that HTTP/HTTPS $ref resolution is supported only when the user
enables --resolve-refs plus --resolve-http-refs, explain that --insecure
disables TLS certificate verification for those HTTP(S) refs and therefore
reduces security (man‑in‑the‑middle risk), and mention that users should prefer
pinned/local refs or validated TLS and use --validate-structure when parsing to
reduce risk.
In `@internal/doctest/doc_test.go`:
- Around line 17-270: Replace raw testing.T assertions in
TestDeepDiveOptionTables with stretchr/testify calls: import
"github.com/stretchr/testify/require" and "github.com/stretchr/testify/assert";
use require.True/NoError (e.g., replace the runtime.Caller nil-check and other
fatal checks with require.True/require.NoError) for preconditions that should
stop the test, and use assert.True/assert.False/assert.Contains with descriptive
messages in place of t.Errorf checks (e.g., replace checks that a function
exists in source/doc like the t.Errorf lines with assert.True(sourceSet[fn],
...) or assert.True(docOpts[fn], ...)); keep t.Skipf for the skip conditions.
Ensure all occurrences of t.Fatal, t.Fatalf, t.Error, t.Errorf in
TestDeepDiveOptionTables (and helper expectations in the same file) are migrated
consistently to require/assert.
In `@overlay/deep_dive.md`:
- Around line 235-258: The documentation moved ToParseResult() out of the
ApplyResult field table; ensure the method ToParseResult() is documented
adjacent to the ApplyResult type and clearly describes conversion to
*parser.ParseResult (symbol ToParseResult), verify the ApplyResult and
DryRunResult tables list the exact fields and types shown (ApplyResult,
DryRunResult, Changes/ProposedChange, StructuredWarnings/ApplyWarnings,
ActionsApplied, ActionsSkipped, WouldApply, WouldSkip), and confirm the
DryRunResult section matches the actual type definitions used in code so the
docs reflect the real symbols and types.
ce1d40b to
32ce228
Compare
There was a problem hiding this comment.
Actionable comments posted: 8
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (12)
generator/deep_dive.md (1)
1385-1417:⚠️ Potential issue | 🟡 MinorFix documentation in deep_dive.md:
GenerateResultstruct contains fields that do not exist in the actual implementation.The documented struct lists fields
GeneratedTypes,GeneratedOperations,LoadTime,GenerateTime,SourceSize, andStatswhich are not present in the actualGenerateResultstruct defined ingenerator/generator.go. Update the documentation to match the actual struct definition, which includesFiles,SourceVersion,SourceOASVersion,SourceFormat,PackageName,Issues,InfoCount,WarningCount,CriticalCount, andSuccess. TheGeneratedFilestruct and parser type names (parser.OASVersion,parser.SourceFormat,parser.DocumentStats) are correctly referenced.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@generator/deep_dive.md` around lines 1385 - 1417, The documentation for GenerateResult is out of sync: remove the non-existent fields GeneratedTypes, GeneratedOperations, LoadTime, GenerateTime, SourceSize, and Stats from the GenerateResult listing in deep_dive.md and ensure the documented struct matches the real implementation by listing Files, SourceVersion, SourceOASVersion (parser.OASVersion), SourceFormat (parser.SourceFormat), PackageName, Issues, InfoCount, WarningCount, CriticalCount, and Success; keep the GeneratedFile section and existing parser type references as-is so the doc matches the actual generator/generator.go types.walker/deep_dive.md (1)
297-305:⚠️ Potential issue | 🟡 MinorAdd
CurrentReffield to theWalkContextfields table.The
CurrentReffield is defined in the source code (walker/context.golines 45-47) and is actively used in the walker package, but it is missing from the table at lines 297-305. The field should be added with a description like: "CurrentRef| Reference info when the current node has a $ref (set whenWithRefTracking()is enabled)".The prose at line 857 is accurate:
CurrentRefis indeed populated inRefHandlercallbacks whenWithRefTracking()is enabled, and the field is properly documented in the source code.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@walker/deep_dive.md` around lines 297 - 305, Add a table row describing the WalkContext field CurrentRef: mention it is populated when the current node has a $ref and is set when WithRefTracking() is enabled (e.g., "CurrentRef | Reference info when the current node has a $ref (set when WithRefTracking() is enabled)"). Update the fields table (near the existing JSONPath/PathTemplate/etc. entries) to include this row so documentation matches the WalkContext definition in walker/context.go and its use in RefHandler callbacks.converter/deep_dive.md (1)
133-165:⚠️ Potential issue | 🟡 MinorMissing
encoding/jsonimport and JSON data written to a.yamlfile in the full program example.The import block at lines 133–139 lists
fmt,log,os, and theconverterpackage, but omits"encoding/json". The call tojson.MarshalIndentat line 163 will therefore not compile if this snippet is used verbatim.Additionally,
json.MarshalIndentalways produces JSON bytes, yet the result is written toopenapi.yaml(line 164). The input file isswagger.yaml(YAML), soresult.SourceFormatwill be YAML; using a JSON marshaler here contradicts the stated format-preservation behaviour described in the Overview section (line 29) and produces a JSON file with a misleading.yamlextension.🛠️ Proposed fix
import ( "fmt" + "encoding/json" "log" "os" "github.com/erraggy/oastools/converter" )For format-preserving output, either use
result.ToParseResult()(which carriesSourceFormat) or branch onresult.SourceFormatto pick the right marshaler:- data, _ := json.MarshalIndent(result.Document, "", " ") - os.WriteFile("openapi.yaml", data, 0644) + // Use ToParseResult() to chain with the parser's format-aware marshaling, + // or branch on result.SourceFormat to select json vs yaml output. + pr := result.ToParseResult() + data, _ := pr.Marshal() // honours SourceFormat (JSON or YAML) + os.WriteFile("openapi.yaml", data, 0644)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@converter/deep_dive.md` around lines 133 - 165, The snippet is missing the "encoding/json" import and writes JSON bytes to a .yaml file; to fix, add the encoding/json import and change the output step to preserve the original format by using result.ToParseResult() or checking result.SourceFormat in main (after converter.ConvertWithOptions) to select the appropriate marshaler (json.MarshalIndent for "json" and a YAML marshaler for "yaml") and write the file with the matching extension (e.g., .json or .yaml) so json.MarshalIndent and result.SourceFormat/ToParseResult are used correctly.joiner/deep_dive.md (3)
1631-1658:⚠️ Potential issue | 🟠 Major
JoinResultstruct reference is missingStructuredWarningsfield.The field is actively used in code examples throughout the documentation (lines 1088–1093, 1163–1167, and in the Handlers section) and exists in the actual Go code. It must be added to the struct documentation at lines 1631–1658:
Proposed struct addition
// Warnings contains non-fatal issues encountered Warnings []string + // StructuredWarnings contains categorized warnings with path and message details + StructuredWarnings JoinWarnings + // CollisionCount tracks resolved collisions🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@joiner/deep_dive.md` around lines 1631 - 1658, The JoinResult struct documentation is missing the StructuredWarnings field used elsewhere; update the struct block for JoinResult to include a StructuredWarnings field (matching the actual Go type, e.g., StructuredWarnings []StructuredWarning) with a brief description like "StructuredWarnings contains machine-friendly warning objects" so the docs align with code and examples referencing StructuredWarnings.
1580-1607:⚠️ Potential issue | 🟠 MajorUpdate
JoinerConfigstruct reference in joiner/deep_dive.md to includeOperationContextandPrimaryOperationPolicyfields.Both fields are defined in
joiner/joiner.goand actively used in code examples throughout the same documentation file (config.OperationContext = trueandconfig.PrimaryOperationPolicy = joiner.PolicyMostSpecific), yet they are missing from the struct reference block. This creates an inconsistency where readers cannot find these exported fields in the documented struct definition.Add the following fields (after the rename-strategy block):
// Rename strategy configuration RenameTemplate string // Go template: "{{.Name}}_{{.Source}}" NamespacePrefix map[string]string // Source file → prefix mapping AlwaysApplyPrefix bool // Apply prefix to all schemas, not just collisions + // Operation-aware renaming + OperationContext bool // Enable operation reference graph for rename templates + PrimaryOperationPolicy PrimaryOperationPolicy // How to select primary op for multi-op schemas + // Equivalence detection configuration EquivalenceMode string // "none", "shallow", or "deep"🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@joiner/deep_dive.md` around lines 1580 - 1607, The JoinerConfig struct documentation is missing two exported fields used elsewhere; update the JoinerConfig reference to add OperationContext (bool) and PrimaryOperationPolicy (type matching joiner.PrimaryOperationPolicy, e.g., an enum/identifier) after the rename-strategy block so examples like config.OperationContext = true and config.PrimaryOperationPolicy = joiner.PolicyMostSpecific match the documented struct; ensure names exactly match the symbols OperationContext and PrimaryOperationPolicy and place them in the same location described in the comment.
1613-1624:⚠️ Potential issue | 🟠 MajorThree additional functional options should be added to the Available Options table.
The options
WithOperationContext(bool),WithPrimaryOperationPolicy(PrimaryOperationPolicy), andWithSourceMaps(map[string]*parser.SourceMap)are implemented injoiner_options.go(lines 513, 524, and 535 respectively) and referenced in this document but missing from the table at lines 1613–1624:
WithOperationContext(bool)— Prose reference at line 657; config field used in examples at lines 424, 611WithPrimaryOperationPolicy(PrimaryOperationPolicy)— Config field used in examples at lines 432, 613WithSourceMaps(map[string]*parser.SourceMap)— Direct usage at line 1701 in the Source Map Integration section🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@joiner/deep_dive.md` around lines 1613 - 1624, The options table is missing three implemented option builders; add rows for WithOperationContext(bool) (toggles operation-scoped context), WithPrimaryOperationPolicy(PrimaryOperationPolicy) (sets primary-operation selection policy), and WithSourceMaps(map[string]*parser.SourceMap) (provides source maps for inputs) to the Available Options table where the other With* options are listed; use the implementations in joiner_options.go (look for WithOperationContext, WithPrimaryOperationPolicy, WithSourceMaps) as authoritative for the exact names and brief descriptions so the table and the prose references (e.g., Source Map Integration) stay consistent.docs/developer-guide.md (2)
729-729:⚠️ Potential issue | 🟡 MinorStale speedup value:
154x fastershould be150x faster.The performance table in
why-oastools.mdrecordsJoinParsed()at150x faster, but Line 729 still reads(154x faster).📝 Proposed fix
-**Joining Pre-Parsed Documents (154x faster):** +**Joining Pre-Parsed Documents (150x faster):**🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/developer-guide.md` at line 729, Update the stale speedup label in the docs: find the heading text "Joining Pre-Parsed Documents (154x faster)" and change the numeric value from "154x faster" to "150x faster" so it matches the performance table entry for JoinParsed(); keep the rest of the heading identical.
297-297:⚠️ Potential issue | 🟡 MinorStale speedup value:
30x fastershould be31x faster.Line 1743 (changed in this PR) correctly uses 31x, and
why-oastools.md's performance table also says 31x, but the comment at Line 297 was not updated and still reads(30x faster).📝 Proposed fix
-// Parse once, validate multiple times (30x faster) +// Parse once, validate multiple times (31x faster)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/developer-guide.md` at line 297, Update the inline comment "Parse once, validate multiple times (30x faster)" to read "(31x faster)" so it matches the updated Line 1743 and the performance table in why-oastools.md; locate the comment text "Parse once, validate multiple times (30x faster)" and change the numeric value to 31.cmd/oastools/commands/walk_operations.go (1)
185-195:⚠️ Potential issue | 🟠 MajorDetail mode should emit the raw Operation node only
operationDetailViewwraps metadata around the operation, which breaks the walk detail contract and prevents--detail --format yamlfrom being a valid OAS fragment. Please render the rawop.Operationdirectly.Proposed fix
- view := operationDetailView{ - Method: strings.ToUpper(op.Method), - Path: op.PathTemplate, - Operation: op.Operation, - } - if err := RenderDetail(os.Stdout, view, flags.Format); err != nil { + if err := RenderDetail(os.Stdout, op.Operation, flags.Format); err != nil { return fmt.Errorf("walk operations: rendering detail: %w", err) }Based on learnings: In the oastools walk command, the detail mode (--detail) should render only the raw OAS node (e.g., op.Operation, info.Schema, info.SecurityScheme) without wrapper metadata.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@cmd/oastools/commands/walk_operations.go` around lines 185 - 195, renderOperationsDetail currently wraps the Operation in operationDetailView before calling RenderDetail, which violates the detail-mode contract; change renderOperationsDetail to call RenderDetail(os.Stdout, op.Operation, flags.Format) (or otherwise pass the raw op.Operation node) for each op so detail output emits the raw OAS Operation node rather than metadata wrappers; update any related uses of operationDetailView in this function accordingly.cmd/oastools/commands/walk_schemas.go (1)
123-133:⚠️ Potential issue | 🟠 MajorDetail mode should emit the raw Schema node only
schemaDetailViewwraps metadata around the schema, which breaks the walk detail contract and prevents--detail --format yamlfrom being a valid OAS fragment. Renderinfo.Schemadirectly.Proposed fix
- view := schemaDetailView{ - Name: info.Name, - JSONPath: info.JSONPath, - IsComponent: info.IsComponent, - Schema: info.Schema, - } - if err := RenderDetail(os.Stdout, view, flags.Format); err != nil { + if err := RenderDetail(os.Stdout, info.Schema, flags.Format); err != nil { return fmt.Errorf("walk schemas: rendering detail: %w", err) }Based on learnings: In the oastools walk command, the detail mode (--detail) should render only the raw OAS node (e.g., op.Operation, info.Schema, info.SecurityScheme) without wrapper metadata.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@cmd/oastools/commands/walk_schemas.go` around lines 123 - 133, The code currently wraps the schema in schemaDetailView before calling RenderDetail, which violates the walk detail contract; change the detail branch so that when flags.Detail is true you call RenderDetail(os.Stdout, info.Schema, flags.Format) (or otherwise render info.Schema directly) for each info in filtered, removing the schemaDetailView wrapper and keeping the existing error handling (adjust the error message context if needed) so the raw Schema node is emitted (symbols: flags.Detail, filtered, info.Schema, schemaDetailView, RenderDetail, flags.Format).docs/index.md (1)
108-111:⚠️ Potential issue | 🟡 MinorUpdate Go install command to the documented entry point
This should use the
cmd/oastoolslatestentry point to match the repository’s install guidance.Proposed fix
-go install github.com/erraggy/oastools/cmd/oastools@latest # Go install +go install github.com/erraggy/oastools/cmd/oastoolslatest@latest # Go installBased on learnings: Install oastools via Homebrew using
brew install erraggy/oastools/oastoolsor from source usinggo install github.com/erraggy/oastools/cmd/oastoolslatest(requires Go 1.24+).🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/index.md` around lines 108 - 111, Update the Go install line in the docs that currently references the wrong package path by replacing the existing command "go install github.com/erraggy/oastools/cmd/oastools@latest" with the documented entry point "go install github.com/erraggy/oastools/cmd/oastoolslatest" so the install guidance matches the repository's intended binary (ensure the README/docs line containing the go install command is updated accordingly).docs/whitepaper.md (1)
2042-2046:⚠️ Potential issue | 🟡 MinorUpdate Go install command to the documented entry point
The repository guidance says to install from source via
cmd/oastoolslatest, but this section usescmd/oastools@latest. Please align the command.Proposed fix
-go install github.com/erraggy/oastools/cmd/oastools@latest +go install github.com/erraggy/oastools/cmd/oastoolslatest@latestBased on learnings: Install oastools via Homebrew using
brew install erraggy/oastools/oastoolsor from source usinggo install github.com/erraggy/oastools/cmd/oastoolslatest(requires Go 1.24+).🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/whitepaper.md` around lines 2042 - 2046, Replace the incorrect install command string "go install github.com/erraggy/oastools/cmd/oastools@latest" in the docs/whitepaper.md code block with the documented entry point path "go install github.com/erraggy/oastools/cmd/oastoolslatest@latest" (or "go install github.com/erraggy/oastools/cmd/oastoolslatest" if you prefer the Go 1.24+ form) so the example matches the repository guidance; optionally add the Homebrew alternative "brew install erraggy/oastools/oastools" on a separate line if you want both install methods documented.
🤖 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 1117-1118: Update the description for WithStdlibRouter() to
disambiguate the "(default)" note: replace the current text with a clearer
sentence such as "Use the stdlib router with PathMatcherSet for routing (the
default when no WithRouter option is provided; calling WithStdlibRouter()
explicitly is a no-op if stdlib routing is already selected)". Make this change
in the documentation entry that lists WithRouter(strategy), WithStdlibRouter(),
and confirm the public type PathMatcherSet remains referenced as-is.
In `@cmd/oastools/commands/cli_doc_test.go`:
- Around line 80-83: The helper mustFS is misleading because it doesn't panic;
rename the function mustFS to a non-asserting name such as fsOf or
extractFlagSet (update the function signature and comment accordingly) and
update all call sites (e.g., in TestCLIFlagsDocumented where you unpack
Setup*Flags() two-value returns) to use the new name; ensure the comment
explains it simply "fsOf extracts the *flag.FlagSet from a Setup*Flags return
pair" to avoid implying a panic.
- Around line 87-121: The sectionMap inside parseDocFlags is an identity mapping
duplicating the authoritative commands set in TestCLIFlagsDocumented (commands)
and can get out of sync; change parseDocFlags signature to accept the caller's
commands map or a slice/set of command names (e.g., add a parameter commands
map[string]bool or []string), remove the hard-coded sectionMap variable, and use
the passed-in commands to build the section lookup when matching headers so
missing/extra commands are detected by the caller's source of truth (update all
call sites, notably the TestCLIFlagsDocumented test that currently builds
commands, to pass that structure into parseDocFlags).
In `@cmd/oastools/commands/walk_parameters.go`:
- Around line 40-42: The flags registration in walk_parameters.go is
inconsistent with the other walk subcommands: change the BoolVar calls to
register "quiet" first and then "q" second, and update the "q" description to
include "(shorthand)" so it matches walk_security.go and walk_paths.go;
specifically modify the fs.BoolVar calls that set flags.Quiet so the long flag
name "quiet" is registered before the shorthand "q" and the shorthand's usage
string reads "Suppress headers and decoration (shorthand)".
In `@converter/deep_dive.md`:
- Around line 354-355: The code currently always uses
json.MarshalIndent(result.Document, "", " ") and writes to files named *.yaml,
which produces JSON bytes for YAML inputs; change the write logic to check
result.SourceFormat (and/or the target filename) and marshal using a YAML
serializer (e.g., yaml.Marshal(result.Document)) when the source/target is YAML,
otherwise use json.MarshalIndent for JSON, then write the resulting bytes with
os.WriteFile; update every occurrence that calls
json.MarshalIndent(result.Document, "", " ") and writes to "swagger.yaml" or
"*-v3.yaml" so they respect result.SourceFormat and produce matching YAML
output.
In `@differ/deep_dive.md`:
- Around line 710-723: Update the practical examples (e.g., the Basic Difference
Detection and Breaking Change Detection examples) to show how to access the new
DiffResult fields: SourceOASVersion, SourceStats, SourceSize, TargetOASVersion,
TargetStats, and TargetSize; demonstrate reading these fields from the
DiffResult value returned by the diff function, display or log the OAS version
values and document statistics (e.g., number of paths/components) and sizes, and
include a short snippet that programmatically compares SourceOASVersion vs
TargetOASVersion to show version-based conditional handling so consumers can see
real usage of these new fields.
In `@internal/doctest/doc_test.go`:
- Around line 43-195: The test's large exception lists (sourceExceptions map)
for "builder" and "walker" hide many public With* options; create a follow-up
tracking issue to document these options and shrink the exception maps over
time, then add a TODO comment in the same file referencing that issue ID next to
the builder and walker entries (the sourceExceptions map and the specific keys
"builder" and "walker" and some example symbols like WithTagDescription,
WithServerDescription, WithHandler, WithDocumentHandler to locate the entries),
and update the staleness check maintenance note so future PRs remove entries as
docs are added.
In `@walker/deep_dive.md`:
- Around line 453-456: The Walk Options configuration block is missing the
non-deprecated option WithMaxSchemaDepth; add an entry for
WithMaxSchemaDepth(depth int) alongside the existing WithMaxDepth(depth int) //
Deprecated: use WithMaxSchemaDepth instead and WithContext(ctx context.Context)
so the docs match the Walk example that uses WithMaxSchemaDepth(50) and the
WalkWithOptions Input Options block; ensure the name and signature match the
actual API (WithMaxSchemaDepth(depth int)).
---
Outside diff comments:
In `@cmd/oastools/commands/walk_operations.go`:
- Around line 185-195: renderOperationsDetail currently wraps the Operation in
operationDetailView before calling RenderDetail, which violates the detail-mode
contract; change renderOperationsDetail to call RenderDetail(os.Stdout,
op.Operation, flags.Format) (or otherwise pass the raw op.Operation node) for
each op so detail output emits the raw OAS Operation node rather than metadata
wrappers; update any related uses of operationDetailView in this function
accordingly.
In `@cmd/oastools/commands/walk_schemas.go`:
- Around line 123-133: The code currently wraps the schema in schemaDetailView
before calling RenderDetail, which violates the walk detail contract; change the
detail branch so that when flags.Detail is true you call RenderDetail(os.Stdout,
info.Schema, flags.Format) (or otherwise render info.Schema directly) for each
info in filtered, removing the schemaDetailView wrapper and keeping the existing
error handling (adjust the error message context if needed) so the raw Schema
node is emitted (symbols: flags.Detail, filtered, info.Schema, schemaDetailView,
RenderDetail, flags.Format).
In `@converter/deep_dive.md`:
- Around line 133-165: The snippet is missing the "encoding/json" import and
writes JSON bytes to a .yaml file; to fix, add the encoding/json import and
change the output step to preserve the original format by using
result.ToParseResult() or checking result.SourceFormat in main (after
converter.ConvertWithOptions) to select the appropriate marshaler
(json.MarshalIndent for "json" and a YAML marshaler for "yaml") and write the
file with the matching extension (e.g., .json or .yaml) so json.MarshalIndent
and result.SourceFormat/ToParseResult are used correctly.
In `@docs/developer-guide.md`:
- Line 729: Update the stale speedup label in the docs: find the heading text
"Joining Pre-Parsed Documents (154x faster)" and change the numeric value from
"154x faster" to "150x faster" so it matches the performance table entry for
JoinParsed(); keep the rest of the heading identical.
- Line 297: Update the inline comment "Parse once, validate multiple times (30x
faster)" to read "(31x faster)" so it matches the updated Line 1743 and the
performance table in why-oastools.md; locate the comment text "Parse once,
validate multiple times (30x faster)" and change the numeric value to 31.
In `@docs/index.md`:
- Around line 108-111: Update the Go install line in the docs that currently
references the wrong package path by replacing the existing command "go install
github.com/erraggy/oastools/cmd/oastools@latest" with the documented entry point
"go install github.com/erraggy/oastools/cmd/oastoolslatest" so the install
guidance matches the repository's intended binary (ensure the README/docs line
containing the go install command is updated accordingly).
In `@docs/whitepaper.md`:
- Around line 2042-2046: Replace the incorrect install command string "go
install github.com/erraggy/oastools/cmd/oastools@latest" in the
docs/whitepaper.md code block with the documented entry point path "go install
github.com/erraggy/oastools/cmd/oastoolslatest@latest" (or "go install
github.com/erraggy/oastools/cmd/oastoolslatest" if you prefer the Go 1.24+ form)
so the example matches the repository guidance; optionally add the Homebrew
alternative "brew install erraggy/oastools/oastools" on a separate line if you
want both install methods documented.
In `@generator/deep_dive.md`:
- Around line 1385-1417: The documentation for GenerateResult is out of sync:
remove the non-existent fields GeneratedTypes, GeneratedOperations, LoadTime,
GenerateTime, SourceSize, and Stats from the GenerateResult listing in
deep_dive.md and ensure the documented struct matches the real implementation by
listing Files, SourceVersion, SourceOASVersion (parser.OASVersion), SourceFormat
(parser.SourceFormat), PackageName, Issues, InfoCount, WarningCount,
CriticalCount, and Success; keep the GeneratedFile section and existing parser
type references as-is so the doc matches the actual generator/generator.go
types.
In `@joiner/deep_dive.md`:
- Around line 1631-1658: The JoinResult struct documentation is missing the
StructuredWarnings field used elsewhere; update the struct block for JoinResult
to include a StructuredWarnings field (matching the actual Go type, e.g.,
StructuredWarnings []StructuredWarning) with a brief description like
"StructuredWarnings contains machine-friendly warning objects" so the docs align
with code and examples referencing StructuredWarnings.
- Around line 1580-1607: The JoinerConfig struct documentation is missing two
exported fields used elsewhere; update the JoinerConfig reference to add
OperationContext (bool) and PrimaryOperationPolicy (type matching
joiner.PrimaryOperationPolicy, e.g., an enum/identifier) after the
rename-strategy block so examples like config.OperationContext = true and
config.PrimaryOperationPolicy = joiner.PolicyMostSpecific match the documented
struct; ensure names exactly match the symbols OperationContext and
PrimaryOperationPolicy and place them in the same location described in the
comment.
- Around line 1613-1624: The options table is missing three implemented option
builders; add rows for WithOperationContext(bool) (toggles operation-scoped
context), WithPrimaryOperationPolicy(PrimaryOperationPolicy) (sets
primary-operation selection policy), and
WithSourceMaps(map[string]*parser.SourceMap) (provides source maps for inputs)
to the Available Options table where the other With* options are listed; use the
implementations in joiner_options.go (look for WithOperationContext,
WithPrimaryOperationPolicy, WithSourceMaps) as authoritative for the exact names
and brief descriptions so the table and the prose references (e.g., Source Map
Integration) stay consistent.
In `@walker/deep_dive.md`:
- Around line 297-305: Add a table row describing the WalkContext field
CurrentRef: mention it is populated when the current node has a $ref and is set
when WithRefTracking() is enabled (e.g., "CurrentRef | Reference info when the
current node has a $ref (set when WithRefTracking() is enabled)"). Update the
fields table (near the existing JSONPath/PathTemplate/etc. entries) to include
this row so documentation matches the WalkContext definition in
walker/context.go and its use in RefHandler callbacks.
---
Duplicate comments:
In @.github/workflows/docs.yml:
- Around line 39-50: The workflow currently inherits a workflow-level
permissions: contents: write that is too broad for the lychee cache and
link-check steps; update the job that contains the "Restore lychee cache" (uses:
actions/cache@v4) and "Check links" (uses: lycheeverse/lychee-action@v2) steps
to include a minimal permissions block like permissions: contents: read at the
job level (or move these steps into a separate job with permissions: contents:
read) so they no longer inherit contents: write.
In `@converter/deep_dive.md`:
- Line 984: The documentation currently mentions using result.ToParseResult() or
marshaling result.Document but doesn't instruct which marshaler to pick; update
the guidance to explicitly recommend selecting a JSON marshaler when
ConversionResult.SourceFormat indicates JSON and a YAML marshaler when it
indicates YAML, and show that choice applies whether chaining via
result.ToParseResult() or when marshaling result.Document directly so readers
know to conditionally choose the correct format based on
ConversionResult.SourceFormat.
In `@differ/deep_dive.md`:
- Line 351: Ensure differ.CategoryExtension remains present in the categoryOrder
slice so extension-related changes are not dropped; update the categoryOrder
definition (where categoryOrder is declared) to include differ.CategoryExtension
if missing, verify the grouping example now includes extension changes, and run
the unit/integration tests that exercise grouping to confirm the fix.
Full documentation accuracy audit across all 11 packages, whitepaper, and site pages. Fixes ~100 inaccuracies (wrong types, missing fields, outdated examples, broken links) and adds automated CI checks to prevent drift from recurring: - lychee link checker in docs workflow and Makefile - TestCLIFlagsDocumented: bidirectional CLI flag/doc verification - TestDeepDiveOptionTables: AST-based With* function/doc verification - Refactor 6 walk subcommands to exported Setup*Flags pattern - Add 12 previously undocumented CLI flags to cli-reference.md Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
erraggy
force-pushed
the
docs/accuracy-audit-ci-checks
branch
from
32ce228 to
ba09be9
Compare
5 tasks
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
1 participant
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.
Summary
Full documentation accuracy audit across all 11 packages, whitepaper, and site pages with automated CI checks to prevent drift from recurring.
deep_dive.mdfiles,docs/whitepaper.md,docs/cli-reference.md, and site pagesmake lint-links/make docs-checktargetsTestCLIFlagsDocumented: bidirectional verification that every CLI flag is documented and every documented flag exists (caught 12 pre-existing undocumented flags)TestDeepDiveOptionTables: AST-based verification that every exportedWith*function appears in its package'sdeep_dive.md, with staleness checks for exception listsSetup*Flags()+ typed flag structs for all 6 walk subcommands, bringing them in line with the other 9 commandsdocs/cli-reference.md(validate, parse, fix, join)Test plan
make checkpasses (8405 tests)TestCLIFlagsDocumented— 15/15 commands passTestDeepDiveOptionTables— 11/11 packages passmake lint-links— 216 links, 0 errors, 50 excluded (config working)🤖 Generated with Claude Code