Skip to content

Fix/regex in path params - #64

Merged
akfaiz merged 4 commits into
oaswrap:mainfrom
GamerGirlandCo:fix/regex-in-path-params
Jul 25, 2026
Merged

Fix/regex in path params#64
akfaiz merged 4 commits into
oaswrap:mainfrom
GamerGirlandCo:fix/regex-in-path-params

Conversation

@GamerGirlandCo

Copy link
Copy Markdown
Contributor

Description

this PR fixes a bug where path/template params with regex constraints (/sub/path/{type:something|orother}) are parsed incorrectly, resulting in validation errors when trying to marshal documents to YAML/JSON. given the previous example, {type:something|orother} would result in a template param with the name type:something|orother, instead of type.

Type of Change

  • Bug fix
  • New feature
  • Breaking change
  • Documentation update
  • Refactor / code quality
  • Other (describe):

Checklist

  • make check passes locally (sync + tidy + lint + test)
  • Tests added or updated to cover the change
  • Golden files updated if spec output changed (make test-update)
  • Relevant documentation updated (README, adapter README, etc.)
  • Commit messages follow Conventional Commits

@codecov

codecov Bot commented Jul 17, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

@llamapreview llamapreview Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

AI Code Review by LlamaPReview

🎯 TL;DR & Recommendation

Recommendation: Approve with suggestions
This PR fixes a bug in path parameter parsing when regex constraints are present. The regex change correctly isolates the parameter name, and tests have been added for the new behavior, though some edge cases remain uncovered.

🌟 Strengths

  • Solid fix: regex correctly isolates param name before :, reducing risk of validation errors.
  • Tests added for primary scenarios.

💡 Suggestions (P2)

  • internal/builder/builder.go: The regex change extracts the param name correctly, but the regex itself is not unit-tested, which could let edge cases (e.g., empty constraints, escaped braces) slip through and cause future bugs.
  • internal/validate/utils_test.go: New tests cover basic regex constraints but omit edge cases like empty constraints and escaped braces, reducing regression confidence.

💡 Have feedback? We'd love to hear it in our GitHub Discussions.
✨ This review was generated by LlamaPReview Advanced, which is free for all open-source projects. Learn more.

Comment thread internal/builder/builder.go Outdated
Comment thread internal/validate/utils_test.go
@GamerGirlandCo
GamerGirlandCo force-pushed the fix/regex-in-path-params branch 3 times, most recently from 65eb795 to 51e0123 Compare July 17, 2026 23:48
@GamerGirlandCo
GamerGirlandCo force-pushed the fix/regex-in-path-params branch from 51e0123 to 9444311 Compare July 18, 2026 00:00
@akfaiz
akfaiz self-requested a review July 25, 2026 10:59
@akfaiz

akfaiz commented Jul 25, 2026

Copy link
Copy Markdown
Member

Hi @GamerGirlandCo , thanks for your contribution.

@akfaiz
akfaiz merged commit e84c03c into oaswrap:main Jul 25, 2026
16 checks passed
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.

2 participants