feat(core)!: shared parser orchestration, unified defaults, codec boundary decode - #745
Merged
Conversation
📝 WalkthroughWalkthroughA new ChangesCore query parsing and error surface
Estimated code review effort: 4 (Complex) | ~60 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Warning Review ran into problems🔥 ProblemsGit: Failed to clone repository. Please run the 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 |
There was a problem hiding this comment.
Pull request overview
This PR centralizes query parsing orchestration in @rapiq/core via a new BaseQueryParser, adds support for JSON-API wire parameter names (filter, page, include) alongside canonical keys, and makes schema defaults consistently apply during full-query parsing (even when parameters are absent). It also tightens the expression dialect with typed syntax errors, correct keyword tokenization, and not() support for match operators, while updating docs/tests and moving URLParameter into core.
Changes:
- Introduces
BaseQueryParser+IQueryParameterParser/ParseQueryOptionsin core, removing duplicated compositeparse()logic and per-packagetypes.ts. - Adds URL-parameter wire-name acceptance with canonical-key precedence; migrates
URLParameterinto@rapiq/coreand updates codec-url-simple to import it from core. - Applies schema defaults during composite parsing and updates expression parsing semantics (typed
SYNTAX_INVALID,not()for match ops, tokenizer fix).
Reviewed changes
Copilot reviewed 28 out of 28 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| packages/parser-simple/test/unit/parser/parser.spec.ts | Adds composite-parse tests for wire names, precedence, defaults, and relations allow-list enforcement. |
| packages/parser-simple/src/types.ts | Removes package-local ParseOptions type (now replaced by core query options types). |
| packages/parser-simple/src/parameter/sorts/module.ts | Aligns sort-default output keys to match input-derived shape (no implicit schema-name prefixing). |
| packages/parser-simple/src/parameter/pagination/module.ts | Treats absent pagination input as non-failure while still applying schema constraints (e.g. maxLimit). |
| packages/parser-simple/src/parameter/filters/module.ts | Treats absent filters input as non-failure so schema defaults can apply. |
| packages/parser-simple/src/module.ts | Switches SimpleParser to extend BaseQueryParser and removes duplicated orchestration. |
| packages/parser-expression/test/unit/parser/parser.spec.ts | Adds composite-parse tests for expression parser (wire names, precedence, allow-list enforcement, defaults). |
| packages/parser-expression/test/unit/parser/filters.spec.ts | Adds tests for not() match negation, keyword-prefixed identifiers, typed syntax errors, and defaults-on-absent. |
| packages/parser-expression/src/types.ts | Removes package-local ParseOptions type (now replaced by core query options types). |
| packages/parser-expression/src/parameter/filters/module.ts | Adds defaults-on-absent behavior, typed syntax errors, tokenizer fix, and not() support for match operators. |
| packages/parser-expression/src/module.ts | Switches ExpressionParser to extend BaseQueryParser and removes duplicated orchestration. |
| packages/docs/integrations/simple.md | Documents wire names and default-application behavior for full-query parsing. |
| packages/docs/integrations/expression.md | Updates expression dialect docs (wire names, not() match negation, SYNTAX_INVALID semantics). |
| packages/core/src/parser/types.ts | Adds ParseQueryOptions and IQueryParameterParser contract in core. |
| packages/core/src/parser/query.ts | Introduces shared composite query parse orchestration with canonical/wire parameter lookup and relations gating. |
| packages/core/src/parser/index.ts | Re-exports the new query parser orchestrator. |
| packages/core/src/errors/parse.ts | Adds ParseError.syntaxInvalid() helper producing ErrorCode.SYNTAX_INVALID. |
| packages/core/src/errors/code.ts | Adds ErrorCode.SYNTAX_INVALID. |
| packages/core/src/constants.ts | Adds URLParameter enum (JSON-API wire names) in core. |
| packages/codec-url-simple/src/index.ts | Stops exporting codec-local constants (URLParameter now comes from core). |
| packages/codec-url-simple/src/encoder/visitors/sort.ts | Switches URLParameter import to @rapiq/core for sort serialization. |
| packages/codec-url-simple/src/encoder/visitors/relations.ts | Switches URLParameter import to @rapiq/core for relations serialization. |
| packages/codec-url-simple/src/encoder/visitors/pagination.ts | Switches URLParameter import to @rapiq/core for pagination serialization. |
| packages/codec-url-simple/src/encoder/visitors/filters.ts | Switches URLParameter import to @rapiq/core for filters serialization. |
| packages/codec-url-simple/src/encoder/visitors/fields.ts | Switches URLParameter import to @rapiq/core for fields serialization. |
| packages/codec-url-simple/src/encoder/serializer/module.ts | Switches URLParameter import to @rapiq/core for serializer wiring. |
| packages/codec-url-simple/src/decoder/module.ts | Switches URLParameter import to @rapiq/core for decode mapping. |
| packages/codec-url-simple/src/constants.ts | Deletes codec-local URLParameter enum (moved to core). |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
12
to
15
| FilterFieldOperator, | ||
| Filters, | ||
| Filters, | ||
| URLParameter, | ||
| } from '@rapiq/core'; |
Comment on lines
15
to
19
| import { | ||
| DEFAULT_ID, | ||
| parseKey, | ||
| URLParameter, | ||
| parseKey, | ||
| } from '@rapiq/core'; |
Owner
Author
|
@coderabbitai review |
✅ Action performedReview finished.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (1)
packages/parser-simple/src/parameter/pagination/module.ts (1)
38-50: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueDuplicate "absent vs present-invalid" check across parsers.
The same
typeof input !== 'undefined' && input !== null && throwOnFailureguard is now duplicated verbatim in bothSimpleFiltersParser.run(packages/parser-simple/src/parameter/filters/module.ts, Lines 83-87) and here. Consider hoisting anisInputAbsent(input)helper (e.g. ontoBaseParserin@rapiq/core) to avoid drift if the semantics need to change again.Also applies to: 81-92
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/parser-simple/src/parameter/pagination/module.ts` around lines 38 - 50, The present-vs-absent input guard is duplicated in pagination parsing and should be centralized to prevent drift. Extract the shared `typeof input !== 'undefined' && input !== null` check into a reusable helper such as `isInputAbsent(input)` on `BaseParser` in `@rapiq/core`, then update `PaginationParser.run` and `SimpleFiltersParser.run` to use that helper before deciding whether to throw `PaginationParseError.inputInvalid()` or continue with `finalizePagination`.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@packages/parser-simple/src/parameter/pagination/module.ts`:
- Around line 38-50: The present-vs-absent input guard is duplicated in
pagination parsing and should be centralized to prevent drift. Extract the
shared `typeof input !== 'undefined' && input !== null` check into a reusable
helper such as `isInputAbsent(input)` on `BaseParser` in `@rapiq/core`, then
update `PaginationParser.run` and `SimpleFiltersParser.run` to use that helper
before deciding whether to throw `PaginationParseError.inputInvalid()` or
continue with `finalizePagination`.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: e96edf8f-bd04-4e33-805b-8a4c63ec3167
📒 Files selected for processing (28)
packages/codec-url-simple/src/constants.tspackages/codec-url-simple/src/decoder/module.tspackages/codec-url-simple/src/encoder/serializer/module.tspackages/codec-url-simple/src/encoder/visitors/fields.tspackages/codec-url-simple/src/encoder/visitors/filters.tspackages/codec-url-simple/src/encoder/visitors/pagination.tspackages/codec-url-simple/src/encoder/visitors/relations.tspackages/codec-url-simple/src/encoder/visitors/sort.tspackages/codec-url-simple/src/index.tspackages/core/src/constants.tspackages/core/src/errors/code.tspackages/core/src/errors/parse.tspackages/core/src/parser/index.tspackages/core/src/parser/query.tspackages/core/src/parser/types.tspackages/docs/integrations/expression.mdpackages/docs/integrations/simple.mdpackages/parser-expression/src/module.tspackages/parser-expression/src/parameter/filters/module.tspackages/parser-expression/src/types.tspackages/parser-expression/test/unit/parser/filters.spec.tspackages/parser-expression/test/unit/parser/parser.spec.tspackages/parser-simple/src/module.tspackages/parser-simple/src/parameter/filters/module.tspackages/parser-simple/src/parameter/pagination/module.tspackages/parser-simple/src/parameter/sorts/module.tspackages/parser-simple/src/types.tspackages/parser-simple/test/unit/parser/parser.spec.ts
💤 Files with no reviewable changes (4)
- packages/codec-url-simple/src/constants.ts
- packages/parser-expression/src/types.ts
- packages/codec-url-simple/src/index.ts
- packages/parser-simple/src/types.ts
…ndary decode BaseQueryParser in @rapiq/core owns the composite parse() body; SimpleParser/ExpressionParser become sub-parser wiring. Parsers are transport-agnostic and read only the canonical Parameter keys — the JSON-API wire names (URLParameter) stay codec-only. URLDecoder is the boundary adapter: decode() now accepts a raw query string or a pre-parsed query object (express req.query), maps the wire names and delegates to a schema-aware SimpleParser (registry via constructor, schema via options). Defaults are owned by the sub-parsers: every sub-parser is always invoked and absent input is not a failure. This applies pagination.maxLimit, fields allowed/default, filters.default and sort.default when the parameter is absent, enforces the relations allow-list in full-query parsing, and drops the schema-name prefix from sort defaults. Expression dialect: not() now wraps match expressions via the NOT_* operators, raw Errors became FiltersParseError.syntaxInvalid (ErrorCode.SYNTAX_INVALID), and the tokenizer no longer splits identifiers starting with a keyword (e.g. "order"). BREAKING CHANGE: schema defaults now apply in composite parsing even when a parameter is absent from the input; URLDecoder performs schema validation when constructed with a registry.
tada5hi
force-pushed
the
feat/parser-orchestration
branch
from
July 6, 2026 13:44
454447c to
fa4a3c6
Compare
tada5hi
changed the title
…n filter input Review feedback (PR #745): the OPTIONS type parameter on BaseQueryParser was never consumed by parse(); an empty filter expression string (?filter=) now surfaces a typed syntax error again instead of silently applying schema defaults.
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)
packages/parser-expression/src/parameter/filters/module.ts (1)
431-442: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winLeftover raw
SyntaxErrorundermines the typed-error refactor.
parseExpressionValue(changed at line 438) andparseMatchExpression(line 354) both delegate tonormalizeValue, which still throws a rawnew SyntaxError('Value can not be normalized.')at line 533. This is inconsistent with the rest of the file's conversion toFiltersParseError.syntaxInvalid(...)/keyValueInvalid(...), meaning callers expecting typedFiltersParseErrorinstances can still receive an untypedSyntaxErrorfrom this codepath.Suggested fix (outside selected range, applies to line 533)
- throw new SyntaxError('Value can not be normalized.'); + throw FiltersParseError.syntaxInvalid('Value can not be normalized.');🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/parser-expression/src/parameter/filters/module.ts` around lines 431 - 442, The typed-error refactor is incomplete because normalizeValue still throws a raw SyntaxError, so callers of parseExpressionValue and parseMatchExpression can receive an untyped exception. Update normalizeValue to throw FiltersParseError.syntaxInvalid or FiltersParseError.keyValueInvalid instead of new SyntaxError, keeping the error shape consistent with the rest of module.ts and preserving the same failure context for token/value normalization.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In `@packages/parser-expression/src/parameter/filters/module.ts`:
- Around line 431-442: The typed-error refactor is incomplete because
normalizeValue still throws a raw SyntaxError, so callers of
parseExpressionValue and parseMatchExpression can receive an untyped exception.
Update normalizeValue to throw FiltersParseError.syntaxInvalid or
FiltersParseError.keyValueInvalid instead of new SyntaxError, keeping the error
shape consistent with the rest of module.ts and preserving the same failure
context for token/value normalization.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 26f636ea-609f-46db-ac40-99f22ea2f05e
📒 Files selected for processing (4)
packages/codec-url-simple/src/encoder/visitors/sort.tspackages/core/src/parser/query.tspackages/parser-expression/src/parameter/filters/module.tspackages/parser-expression/test/unit/parser/filters.spec.ts
✅ Files skipped from review due to trivial changes (1)
- packages/codec-url-simple/src/encoder/visitors/sort.ts
🚧 Files skipped from review as they are similar to previous changes (2)
- packages/parser-expression/test/unit/parser/filters.spec.ts
- packages/core/src/parser/query.ts
Closed
This was referenced Jul 7, 2026
This was referenced Jul 15, 2026
Merged
Merged
This was referenced Jul 20, 2026
This was referenced Aug 10, 2026
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
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
Implements plan 006 (parser orchestration, roadmap M2), following the IR-centric layering now recorded in
.agents/architecture.md: parsers transform dialect input to the IR (QueryAST) and are transport-agnostic; codecs own the wire format end to end.Shared orchestration
BaseQueryParserin@rapiq/core(src/parser/query.ts) owns the single compositeparse()body;SimpleParser/ExpressionParserare now sub-parser wiring only (their duplicated ~180-line bodies are gone).IQueryParameterParsercontract andParseQueryOptionstype in core; the identical privatetypes.tsfiles of both parser packages are deleted.Parameterkeys — no transport naming in core or the parser packages.Codec as boundary adapter
URLParameterstays exclusively in@rapiq/codec-url-simple.URLDecoder.decode(input, options?)now accepts a raw query string or a pre-parsed query object (expressreq.query), maps the JSON-API wire names (filter,page,include, …) to canonical parameters and delegates to a schema-awareSimpleParser— registry via constructor,schemavia options. Its duplicated five-block decode body is gone too.decode*methods now forward{ schema }options.Unified defaults
pagination.maxLimitnot applied whenpageabsent, allowed-only fields schemas not projected (v1 parity), filters/sort defaults skipped for non-object input,throwOnFailuretripping on absent parameters.Expression dialect
not()wraps match expressions via the existingNOT_CONTAINS/NOT_STARTS_WITH/NOT_ENDS_WITHoperators.Errorthrows becameFiltersParseError.syntaxInvalid(...)with newErrorCode.SYNTAX_INVALID.orderpreviously tokenized asor+derand failed).Docs & tests
packages/docs/integrations/{simple,expression,url}.mdupdated: transport-agnostic parsers,req.querydecoding via the codec, defaults behavior,not()/ error semantics. The outdated “decode does not validate” warning is replaced by the schema-aware decoder.decoder.spec.tsin the codec (string input, object input, string/object equivalence, schema validation); expression specs for negated matches, keyword-prefixed fields, typed syntax errors..agents/architecture.mdgains a “Layering Principles (IR-centric)” section; plan 006 marked done with the revised item-1 resolution.Breaking changes
maxLimitechoes intoQuery.pagination, allowed-only field schemas produce an explicit projection).URLDecodervalidates against a schema when constructed with aSchemaRegistryand given aschemaoption (previously it never validated).Verification
nx run-many -t build— 7 projects greennx run-many -t test— 385 tests across 6 projects greengrep -rn 'todo: parse parameter' packages/— empty (plan acceptance criterion)Summary by CodeRabbit
not(...)support forcontains,startsWith, andendsWith.