Skip to content

Implement CompiledAnnotation IR struct - #114

Open
dmartmillan wants to merge 5 commits into
milestone/v2.0.0from
65-implement-compiledannotation-ir-struct
Open

dmartmillan wants to merge 5 commits into
milestone/v2.0.0from
65-implement-compiledannotation-ir-struct

Conversation

@dmartmillan

@dmartmillan dmartmillan commented Sep 24, 2026 •

Copy link
Copy Markdown
Collaborator

This pull request corresponds to the implementation of CompiledAnnotation IR struct and the implementation of CompiledLambda struct along with its tests. Relates to #65 issue.

Implemented the different data structures that form the Intermediate Representation (IR), which is an internal data structure that Rust will use to compile and represent the bridge between the high-level source (config that comes from YAML annotation file) and the low-level target machine code (the compiled annotation). The implementation that is in charge to compile the annotation file using IR will be developed on the Epic 2.

On this PR has been:

  • Added ir.rs which encompasses CompiledAnnotation (which is the IR of the annotation file) and CompiledLambda (which is the IR of the function value on the annotation file).

In contrast to the Python-based implementation, the new version of OpenVariant uses the Rhai language for user-defined functions instead of Python lambda functions. Rhai is an embedded scripting language that can be interpreted directly by Rust. Therefore, users must follow Rhai’s syntax and semantics, as described in its documentation: https://rhai.rs/book/ref/values-and-types.html.

Dipping on Rhai closures, I discovered a crazy replace operation, these two lines do the same:
|y| y._upper() ?? y.replace("CHR", "") ?? y
|y| y.to_upper() - "CHR"

CompiledLambda::compile uses Rhai’s compile_expression, which accepts only a single parameter (eg. |x| x.replace("old", "new") ). It does not support let statements, semicolon-separated statements, or {} blocks. Consequently, there is no way to express a sequence such as “do this, then do that, then return y” using ordinary statement syntax. The function must instead be expressed as a single valid Rhai expression.

The Python implementation performs this work for every row of every file: it re-reads the YAML configuration, re-instantiates the builder classes, and calls eval(self.func)(x) on the lambda string each time. In the new implementation, this expensive work will be performed exactly once, at load time.

The resulting IR is plain data composed of enums, strings, pre-compiled regular expressions, and hash maps. It contains no file handles or Python objects. As a result, a single CompiledAnnotation can be wrapped in Arc and safely shared across the threads, avoiding repeated parsing, object construction, and compilation.

  • Added its test at tests/test_annotation/test_ir.rs
  • Added a new entry on Makefile to perform specific test with a filter for example: make test-filter FILTER=test_ir, it will only perform test_ir.

@dmartmillan dmartmillan self-assigned this Sep 24, 2026
@dmartmillan dmartmillan added enhancement New feature or request test Tests relate labels Sep 24, 2026
@dmartmillan
dmartmillan requested a lite review from Copilot September 24, 2026 09:13

Copilot AI 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.

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

Copilot AI 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.

Copilot review overview

🟡 Changes recommended

Critical test issues and moderate validator gaps remain unresolved.

Review effort: Lite
Findings: 1 High severity · 1 Low severity

Open (2)

Comment thread src/tests/test_annotation/test_ir.rs Outdated
#[test]
fn upper_then_replace() {
let lambda = CompiledLambda::compile(
r#"|y| y.make_upper() ?? y.replace("CHR", "") ?? y"#,

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

This is wrong. replace method is a mutating method, in this case mutates that temporary in place and evaluates to () then needs a ?? operator:
a() ?? b(); // b() is only evaluated if a() returns ()
also can be used for:
a ?? b // returns 'a' if it is not (), otherwise 'b'

Comment thread src/annotation/ir.rs Outdated
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request test Tests relate

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants