Skip to content

Refactor/phase 9 final#235

Closed
its-hammer-time wants to merge 14 commits intopb33f:mainfrom
its-hammer-time:refactor/phase-9-final
Closed

Refactor/phase 9 final#235
its-hammer-time wants to merge 14 commits intopb33f:mainfrom
its-hammer-time:refactor/phase-9-final

Conversation

@its-hammer-time
Copy link
Contributor

No description provided.

its-hammer-time and others added 14 commits January 20, 2026 14:37
… consolidate operationForMethod

Phase 0+1 of architecture redesign:

Phase 0 (baseline infrastructure):
- Add Makefile with test, lint, benchmark targets and benchstat support
- Add comprehensive benchmark suite (path matching, request/response
  validation, concurrent validation, memory profiling)
- Capture baseline benchmark results
- Add architecture review and redesign plan documentation

Phase 1 (path matcher chain):
- Add OperationForMethod helper to consolidate 4 duplicated HTTP method
  switch statements (ExtractOperation, ExtractParamsForOperation,
  ExtractSecurityForOperation, pathHasMethod)
- Add LookupWithParams to radix Tree and PathTree for parameter extraction
  during path matching (lazy map allocation, proper backtracking)
- Introduce pathMatcher interface and resolvedRoute type for composable
  path matching
- Implement radixMatcher (O(k) with param extraction) and regexMatcher
  (complex pattern fallback) as chainable matchers
- Build default matcher chain [radixMatcher, regexMatcher] in
  NewValidatorFromV3Model
- Add resolvePath method on validator that uses the matcher chain,
  replacing direct paths.FindPath calls
- Extract FindPathRegex from FindPath for use by the regex matcher
- All changes are internal (unexported types) maintaining full backward
  compatibility

No benchmark regressions. All 1,159+ tests pass with race detector.

Co-authored-by: Cursor <cursoragent@cursor.com>
…on float

Phase 2 of architecture redesign:

- Add `version float32` field to requestBodyValidator, responseBodyValidator,
  and validator structs, computed once at construction via
  helpers.VersionToFloat(document.Version). Replaces per-request
  VersionToFloat calls in validate_body.go (request + response).

- Define requestContext struct carrying per-request shared state: request,
  resolvedRoute, operation, parameters, security, stripped path, segments,
  and cached version. Created once per request to eliminate redundant
  extraction across validators.

- Add buildRequestContext method on validator that strips the path, matches
  via the matcher chain, resolves the operation, extracts parameters and
  security requirements — all exactly once. Returns requestContext or
  validation errors for path-not-found / method-not-found.

- Add comprehensive tests for buildRequestContext covering success paths,
  error paths, parameter extraction, security extraction, version caching
  (3.0 vs 3.1), and path segment splitting.

No behavioral changes — buildRequestContext is defined but not yet wired
into the validation pipeline (Phase 3 threads it through the sync path).
All 1,159+ tests pass with race detector. No benchmark regressions.

Co-authored-by: Cursor <cursoragent@cursor.com>
Phase 3 of architecture redesign:

- Change validationFunction type from func(request, pathItem, pathValue)
  to func(ctx *requestContext), enabling context-based validation flow

- Add 6 wrapper methods on validator (validatePathParamsCtx,
  validateQueryParamsCtx, validateHeaderParamsCtx, validateCookieParamsCtx,
  validateSecurityCtx, validateRequestBodyCtx) that adapt requestContext
  to the existing WithPathItem parameter validator methods

- Add validateRequestSync method that runs all validation functions
  sequentially using the request context

- Modify ValidateHttpRequestSync to use buildRequestContext once, then
  pass context to validateRequestSync (replaces resolvePath + sequential
  WithPathItem calls)

- Modify ValidateHttpRequestSyncWithPathItem to construct a partial
  requestContext from provided arguments and delegate to validateRequestSync

- Update async path in ValidateHttpRequestWithPathItem to construct
  requestContext and use wrapper methods (channel/goroutine pattern
  unchanged — Phase 4 will simplify it)

- Replace all resolvePath calls in ValidateHttpResponse,
  ValidateHttpRequestResponse, and ValidateHttpRequest with
  buildRequestContext

- Remove resolvePath method and unused paths import (path stripping now
  happens exclusively in buildRequestContext)

Net -33 lines. All 1,159+ tests pass with race detector.
No benchmark regressions.

Co-authored-by: Cursor <cursoragent@cursor.com>
…ntext

Phase 4 of architecture redesign:

- Add validateWithContext method that runs all 6 validation functions
  concurrently using sync.WaitGroup + sync.Mutex, replacing the previous
  9-goroutine/5-channel choreography

- Simplify ValidateHttpRequestWithPathItem from ~106 lines of channel
  orchestration code to ~12 lines: construct requestContext, delegate
  to validateWithContext

- Update ValidateHttpRequest to call validateWithContext directly for
  body-bearing requests instead of routing through
  ValidateHttpRequestWithPathItem

- Remove runValidation function (old channel-based orchestration)

- Remove validationFunctionAsync type (no longer needed)

Net -89 lines. All tests pass with race detector (zero data races).
Sync path benchmarks ~34% faster than async for equivalent validation.

Co-authored-by: Cursor <cursoragent@cursor.com>
Phase 5 of architecture redesign:

- Add extractPathParams helper that uses BraceIndices to find parameter
  names in each path template segment and maps them to the corresponding
  request path segment values

- Update regexMatcher.Match to call extractPathParams after FindPathRegex
  finds a match, populating resolvedRoute.pathParams

- Both radix and regex matchers now extract path params, so path parameter
  validation can use the fast path regardless of which matcher hit

- Lazy map allocation: params map only created when first parameter segment
  is encountered, avoiding allocation for literal paths

- Handles edge cases: params with custom patterns ({id:[0-9]+}), no params,
  segment count mismatch, empty names

- Add comprehensive tests: TestRegexMatcher_ExtractsMultipleParams,
  TestRegexMatcher_NoParams, TestExtractPathParams (6-case table test)

All tests pass. No benchmark regressions.

Co-authored-by: Cursor <cursoragent@cursor.com>
Phase 6 of architecture redesign:

- Add WithLazyErrors() option to config.go that enables deferred population
  of expensive error fields (ReferenceSchema, ReferenceObject)

- Add lazySchemaSource type to SchemaValidationFailure with sync.Once
  guards for thread-safe lazy resolution

- Add GetReferenceSchema() and GetReferenceObject() getter methods that
  work in both eager (default) and lazy modes, resolving on first call
  when lazy mode is enabled

- Add SetLazySource() method for validation code to configure lazy
  resolution data without eagerly computing string representations

- Replace json.MarshalIndent with json.Marshal for ReferenceObject in
  both requests/validate_request.go and responses/validate_response.go
  (reduces formatting overhead per error even in eager mode)

- When LazyErrors is enabled, skip json.Marshal and string copy during
  error construction entirely — defer to getter methods

- Mark ReferenceSchema and ReferenceObject fields as Deprecated in favor
  of the new getter methods

Default behavior is unchanged (eager mode). All existing tests pass.
No benchmark regressions.

Co-authored-by: Cursor <cursoragent@cursor.com>
Phase 7 of architecture redesign:

- Replace goroutine/channel pattern in LocateSchemaPropertyNodeByJSONPath
  with plain defer/recover (removes 2 channel allocs + goroutine spawn per
  schema error)

- Add ShouldIgnoreError() and ShouldIgnorePolyError() string-based
  functions to replace IgnoreRegex.MatchString() across all 5 call sites
  (requests, responses, parameters, schema_validation/validate_schema,
  schema_validation/validate_document). Keep deprecated regex vars for
  backward compatibility with external consumers.

- Fix GoLow().Hash() double-compute in requests/validate_request.go,
  responses/validate_response.go, and parameters/validate_parameter.go —
  compute hash once and reuse for both cache-check and cache-store paths

- Fix pre-existing ineffectual assignment of jsonSchema in
  parameters/validate_parameter.go (linter fix)

All 16 test packages pass. No benchmark regressions.

Co-authored-by: Cursor <cursoragent@cursor.com>
…tions

Phase 8 of architecture redesign:

- Change ValidateRequestSchemaInput.Options and
  ValidateResponseSchemaInput.Options from []config.Option to
  *config.ValidationOptions. Internal callers now pass the options struct
  directly, eliminating the allocate-then-discard overhead where
  NewValidationOptions() eagerly created DefaultCache + sync.Map only to
  have WithExistingOpts immediately overwrite them. Saves 6 allocs per
  body validation call.

- Fix WithExistingOpts to use struct dereference (*o = *options) instead
  of field-by-field copy. Prevents silent field-loss bugs when new fields
  are added to ValidationOptions. Keep the function for external consumers.

- Update all internal callers (requests/validate_body.go,
  responses/validate_body.go) and test call sites to use the new direct
  options passing.

All existing tests pass. Benchmark shows -6 allocs/op per validation.

Co-authored-by: Cursor <cursoragent@cursor.com>
Phase 9 of architecture redesign:

- Run final statistical benchmarks (count=5) and generate benchstat
  comparison against Phase 0 baseline

- Add Section 10 "Architecture Redesign Results" to
  docs/architecture-review.md with:
  - Summary table of all 8 implementation phases
  - Benchstat latency comparison (POST sync path -33%, Petstore -11.5%)
  - Benchstat allocation comparison (-8 allocs/op for body validation)
  - Benchstat memory comparison (-10% for Petstore, -5.7% for BulkActions)
  - Analysis of GET path regression (+5 allocs, acceptable tradeoff)
  - Remaining optimization opportunities

- Save final benchmark results to benchmarks/results/current.txt

All existing tests pass. No code changes in this phase.

Co-authored-by: Cursor <cursoragent@cursor.com>
@codecov
Copy link

codecov bot commented Feb 10, 2026

Codecov Report

❌ Patch coverage is 83.05344% with 111 lines in your changes missing coverage. Please review.
✅ Project coverage is 95.78%. Comparing base (85b4a06) to head (c93d634).

Files with missing lines Patch % Lines
paths/paths.go 36.36% 35 Missing ⚠️
errors/validation_error.go 0.00% 34 Missing ⚠️
helpers/ignore_regex.go 0.00% 23 Missing ⚠️
config/config.go 38.46% 8 Missing ⚠️
parameters/validate_parameter.go 96.59% 2 Missing and 1 partial ⚠️
radix/tree.go 98.21% 2 Missing ⚠️
requests/validate_request.go 92.30% 1 Missing and 1 partial ⚠️
responses/validate_response.go 92.30% 1 Missing and 1 partial ⚠️
helpers/parameter_utilities.go 80.00% 1 Missing ⚠️
path_matcher.go 98.03% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #235      +/-   ##
==========================================
- Coverage   97.62%   95.78%   -1.84%     
==========================================
  Files          56       61       +5     
  Lines        5256     5646     +390     
==========================================
+ Hits         5131     5408     +277     
- Misses        125      235     +110     
- Partials        0        3       +3     
Flag Coverage Δ
unittests 95.78% <83.05%> (-1.84%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant