transpile-chain LEG 3 (re-land) + codex P1: ruff rev-pin#150
Conversation
…f methods (transpile-chain LEG 3)
The render end of the transpile chain. New render_class_with_methods(class,
mask, actions): a compile-time (askama = ERB analog) transpiler emitting a
Rust struct whose FIELDS are the ClassView x FieldMask projection (the
bitmask indexes the ObjectView N3 order - attributes then family edges - the
basis OgarClassView::render_rows uses) and whose METHODS are the OGAR
ActionDef DO-arm, as a struct-of-methods constructor (impl { new(..) + one fn
per ActionDef }).
Operator rulings (2026-07-04):
- Behaviour is Rust methods, NOT SurrealQL DDL. The deprecated SurrealQL-AST
adapter (DEFINE EVENT ... WHEN ... THEN ...) is not a target; consistent
with SURREAL-AST-AS-ADAPTER.md. Tests assert no DEFINE EVENT / DEFINE TABLE.
- on_enter (the Rubicon state mutation) -> method takes &mut self; read
actions take &self.
New dep lance-graph-contract (FieldMask, branch=main). 6 new tests (mask
gates fields; ActionDef->methods; dotted-predicate + PascalCase sanitisers);
50 tests green; workspace check clean; clippy -D warnings clean. End-to-end
verified: masked account.move (mask={0,2}) -> struct AccountMove { name,
state } + fn new(name, state) + fn action_post(&mut self), CLASS_ID=0x0202.
Also fixes a pre-existing clippy::cloned_ref_to_slice_refs (1.95 toolchain)
in the list_view test. Ledger: docs/DISCOVERY-MAP.md
D-OGAR-RENDER-CLASSVIEW-FIELDMASK-METHODS.
ogar-from-ruff reads ruff_spo_triplet::Model::inherits (added in ruff #40). With no committed Cargo.lock, the floating `branch = "main"` pin could resolve to a ruff rev WITHOUT that field on a clean build and fail to compile — and the prior commit's '61ce2b49' claim didn't travel. Pin all three ruff refs (ruff_spo_triplet + ruff_spo_address in ogar-from-ruff, ruff_ruby_spo in ogar-from-rails) to the exact rev 61ce2b49 (ruff main at the #40 merge). Same rev across both crates: they share ruff_spo_triplet internally, so a mismatch would double-check-out ruff. Verified: ruff re-locks to 61ce2b49; ogar-from-ruff 48 tests + ogar-from-rails green; workspace check + clippy -D warnings clean.
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 8d02fd4c5e
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if mask.0 == u64::MAX { | ||
| true |
There was a problem hiding this comment.
When callers render a wide harvested class with FieldMask(u64::MAX) (the docs already call out Odoo account.move-sized shapes), this branch emits every slot past position 63 even though the FieldMask contract and ClassView::render_rows treat n >= FieldMask::MAX_FIELDS as absent. That makes the generated struct diverge from the actual ClassView × FieldMask projection and can hide the over-wide-class failure that should be handled by paging/widening or rejection instead of silently generating unreachable fields.
Useful? React with 👍 / 👎.
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes using high effort and found 3 potential issues.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 8d02fd4. Configure here.
| body_comment, | ||
| mutates: a.on_enter.is_some(), | ||
| } | ||
| } |
There was a problem hiding this comment.
Colliding action method names
Medium Severity
The lift_method function uses sanitize_ident to create Rust method names from ActionDef predicates. This process can generate identical names for distinct predicates, leading to duplicate pub fn definitions in the generated impl block. This includes conflicts when a predicate sanitizes to new, clashing with the auto-generated constructor. The resulting Rust code fails to compile.
Reviewed by Cursor Bugbot for commit 8d02fd4. Configure here.
| format!("// ported from source: {}", one_line(first)) | ||
| } | ||
| _ => format!("// TODO: port `{}` from {}", a.predicate, a.object_class), | ||
| }; |
There was a problem hiding this comment.
TODO comment allows code injection
Medium Severity
When body_source is absent, lift_method embeds raw predicate and object_class into a // TODO line without newline sanitization. A multiline predicate ends the line comment after the first line and injects further tokens into the method body, yielding invalid or unintended Rust in generated output.
Reviewed by Cursor Bugbot for commit 8d02fd4. Configure here.
| } else if out.as_bytes()[0].is_ascii_digit() { | ||
| out.insert(0, '_'); | ||
| } | ||
| out |
There was a problem hiding this comment.
Invalid struct name Self
Low Severity
Struct names come from pascal_type_name on class.name without keyword escaping. A class named self or Self becomes the type identifier Self, and the askama template emits pub struct Self, which Rust rejects as an invalid struct name.
Additional Locations (1)
Reviewed by Cursor Bugbot for commit 8d02fd4. Configure here.
2 participants
Follow-up to #149 (merged). #149 landed only LEG 1 + LEG 2 (the ruff→OGAR harvest+lift); the render arm (LEG 3) and the codex pin fix did not make it into
main. This re-lands both, rebased onto mergedmain.LEG 3 (re-landed) —
ogar-render-askama: ClassView × FieldMask → struct + ActionDef methodsrender_class_with_methods(class, mask, actions)— a compile-time (askama =the ERB/XSLT analog) transpiler emitting a Rust struct whose fields are the
ClassView × FieldMaskprojection (the bitmask indexes the ObjectView N3 order— attributes then family edges — the basis
OgarClassView::render_rowsuses;bit
nset ⇒ n-th field emits) and whose methods are the OGARActionDefDO-arm, assembled as a struct-of-methods constructor (
impl { new(..) + one fn per ActionDef }).Operator rulings baked in: behaviour is Rust methods, not SurrealQL DDL
(deprecated AST adapter; tests assert no
DEFINE EVENT/DEFINE TABLE);ActionDef::on_enter(the Rubicon state mutation) ⇒ method takes&mut self.End-to-end verified: a masked
account.move(mask{0,2}) →struct AccountMove { name, state }(field 1amount_totaldropped),fn new(name, state),fn action_post(&mut self),CLASS_ID = 0x0202.Ledger:
D-OGAR-RENDER-CLASSVIEW-FIELDMASK-METHODS.codex P1 fix — rev-pin ruff (was floating
branch = "main")Codex flagged on #149:
ogar-from-ruffreadsruff_spo_triplet::Model::inherits(added in ruff #40), but pinned ruff via floating
branch = "main"with nocommitted
Cargo.lock— a clean build could resolvemainto a rev withoutthe field and fail to compile. All three ruff refs are now pinned to the exact
rev
61ce2b49(ruffmainat the #40 merge):ogar-from-ruff:ruff_spo_triplet,ruff_spo_addressogar-from-rails:ruff_ruby_spoPinned to the same rev because the crates share
ruff_spo_tripletinternally — a rev mismatch would double-check-out ruff and conflict. (The
underlying non-reproducibility is F3: OGAR gitignores
Cargo.lock. This is thesurgical fix; committing the lockfile is a separate repo-policy call.)
Testing
ogar-from-ruff48 tests,ogar-from-railsgreen,ogar-render-askama50tests — all green. ruff re-locks to
61ce2b49cleanly (confirmsModel::inheritspresent at that rev).cargo checkclean;cargo clippy -p ogar-from-ruff -p ogar-from-rails -p ogar-render-askama -- -D warningsclean.🤖 Generated with Claude Code
https://claude.ai/code/session_01EYvNjD8M8LMNYbRy3gq2FP
Generated by Claude Code