feat(catalog/op_bridge): D-AR-6.3 — lower ast::FieldDefinition.assert to catalog Expr (codex P2 from #38)#39
Conversation
|
Warning Review limit reached
More reviews will be available in 58 minutes and 52 seconds. Learn how PR review limits work. Your organization has used up its prepaid credits, and credit purchases are no longer available. Enable the review add-on in the billing tab to keep reviews running — you're only billed for reviews past your plan's rate limits ($0.25/file). ⌛ How to resolve this issue?After more reviews become available, 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 include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
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 |
… to catalog Expr (codex P2 PR #38) P2 review comment on #38: the bridge transferred name/table/kind from `ast::FieldDefinition` but silently dropped `assert: Option<String>` (added in nexgen-rs#27). Result: the catalog accepts values that the rendered SurrealQL announces it would reject — exactly the kind of declared-vs-enforced drift the C16c bridge exists to prevent. # Fix `From<ast::FieldDefinition> for catalog::FieldDefinition` now calls `with_assert(rails_assert_to_expr(s))` so the catalog carries the same constraint the AST renders. # `rails_assert_to_expr` — structural lowering The OpenProject AR-shape extractor emits exactly one expression today: `$value != NONE` (the schema-level marker for any `validates_constraint` triple; see PR #28 fix for why it's not ASSERT-on-`normalizes_attribute`). We construct it structurally as Expr::Binary { left: Param("value"), op: NotEqual, right: Literal::None, } instead of going through surrealdb-core's SurrealQL parser (which is `async` + heavy). One known shape, one explicit arm. Comment stripping: PR #28's `normalize:` annotation renders as `$value != NONE /* normalized */`. The block-comment + whitespace canonicalisation drops the marker before matching so both inputs lower to the same Expr. The marker is metadata; the assertion is unchanged. Unknown strings (e.g. a future `$value > 100 AND $value < 1000`) return `None` — the catalog stays accept-any, matching the previous (pre-D-AR-6.3) silent-drop behaviour. A future PR swapping in a real mini-parser is the natural next step when the AR-shape vocab needs richer expressions (`validates :len, length: {minimum: 3}` → `string::len($value) >= 3`). # Tests +4 new under `--features op-bridge`: - `d_ar_6_3_field_definition_bridges_assert_clause` — matches the structural Binary($value, !=, NONE) shape. - `d_ar_6_3_assert_normalized_comment_is_stripped` — the `/* normalized */` marker is metadata; the lowered Expr is the same as the bare `$value != NONE` case. - `d_ar_6_3_no_assert_when_ast_field_has_none` — passthrough preserves None. - `d_ar_6_3_unknown_assert_string_lowers_to_none` — unknown expressions don't corrupt the catalog (safety net). # Iron-rule lock §0 ANTI-INVENTION GUARDRAIL honoured: one new free fn (`rails_assert_to_expr`) over the existing `Expr` / `Param` / `Literal` types. No new variant, no new builder, no new dep.
AdaWorldAPI
force-pushed
the
claude/op-bridge-assert-field
branch
from
04fd090 to
1998d10
Compare
2 participants
Summary
Follow-up to the merged
AdaWorldAPI/surrealdb#38. Addresses theP2 codex review comment: the bridge transferred name/table/kind
from
ast::FieldDefinitionbut silently droppedassert: Option<String>(added inAdaWorldAPI/openproject-nexgen-rs#27). Result: the catalogaccepted values that the rendered SurrealQL announces it would
reject — exactly the declared-vs-enforced drift the C16c bridge
exists to prevent.
Fix
From<ast::FieldDefinition> for catalog::FieldDefinitionnowcalls
with_assert(rails_assert_to_expr(s))so the catalogcarries the same constraint the AST renders.
rails_assert_to_expr— structural loweringThe OpenProject AR-shape extractor emits exactly one expression
today:
$value != NONE(the schema-level marker for anyvalidates_constrainttriple; see #28 fix for why it's not onnormalizes_attribute). We construct it structurally:instead of going through surrealdb-core's SurrealQL parser
(which is
async+ heavy). One known shape, one explicit arm.Comment stripping
PR #28's
normalize:annotation renders as$value != NONE /* normalized */. The block-comment +whitespace canonicalisation drops the marker before matching, so
both inputs lower to the same
Expr. The marker is metadata; theassertion semantic is unchanged.
Unknown expressions fall back to
NoneFuture Rails expressions (e.g.
$value > 100 AND $value < 1000from
validates :score, numericality:) currently returnNone— the catalog stays accept-any, matching the previous
(pre-D-AR-6.3) silent-drop behaviour. A future PR swapping in a
real mini-parser is the natural next step when the AR-shape vocab
needs richer expressions
(
validates :len, length: {minimum: 3}→string::len($value) >= 3).Tests
+4 new under
--features op-bridge:d_ar_6_3_field_definition_bridges_assert_clause— matches thestructural
Binary($value, !=, NONE)shape.d_ar_6_3_assert_normalized_comment_is_stripped— the/* normalized */marker is metadata; loweredExpris thesame as the bare case.
d_ar_6_3_no_assert_when_ast_field_has_none— passthroughpreserves
None.d_ar_6_3_unknown_assert_string_lowers_to_none— unknownexpressions don't corrupt the catalog (safety net).
Iron-rule lock
§0 ANTI-INVENTION GUARDRAIL honoured: one new free fn
(
rails_assert_to_expr) over the existingExpr/Param/Literaltypes. No new variant, no new builder, no new dep.Test plan
cargo check -p surrealdb-core --features op-bridge→ cleancargo test -p surrealdb-core --features op-bridge -- catalog::op_bridge::tests::d_ar_6_3_(test link disk-bound in current env; lib check verified the types compose)