Skip to content

Add syntax for declarative record descriptors#233

Open
Schamper wants to merge 1 commit into
mainfrom
declarative
Open

Add syntax for declarative record descriptors#233
Schamper wants to merge 1 commit into
mainfrom
declarative

Conversation

@Schamper

@Schamper Schamper commented Jul 1, 2026

Copy link
Copy Markdown
Member

This resolves #195.

Disclaimer: this is mostly vibed but with good steering, iteration and a manual check of the code afterwards.

Instead of completely revamping the type system of flow.record, I opted for it to just be a flavor on top of the existing code. That way it should retain complete backwards compatibility. Then in the future we can decide how much of the original code we would want to change towards this style.

Example from the README, showing various options:

class HttpRequestRecord(RecordBase, name="http/request"):
    ts: datetime
    url: str  # native types map (e.g. str -> string)
    status: Annotated[int, "uint32"]  # or field(typename="uint32")
    remote: str

Using docstrings also "works" and has a helper to retrieve them.

@Schamper
Schamper requested a review from yunzheng July 1, 2026 14:48
@codecov

codecov Bot commented Jul 1, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 0% with 195 lines in your changes missing coverage. Please review.
✅ Project coverage is 0.00%. Comparing base (e6b58fd) to head (d50435f).

Files with missing lines Patch % Lines
flow/record/declarative.py 0.00% 189 Missing ⚠️
flow/record/base.py 0.00% 6 Missing ⚠️

❗ There is a different number of reports uploaded between BASE (e6b58fd) and HEAD (d50435f). Click for more details.

HEAD has 2 uploads less than BASE
Flag BASE (e6b58fd) HEAD (d50435f)
unittests 14 12
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #233       +/-   ##
==========================================
- Coverage   84.29%   0.00%   -84.30%     
==========================================
  Files          35      36        +1     
  Lines        3756    3945      +189     
==========================================
- Hits         3166       0     -3166     
- Misses        590    3945     +3355     
Flag Coverage Δ
unittests 0.00% <0.00%> (-84.30%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@codspeed-hq

codspeed-hq Bot commented Jul 1, 2026

Copy link
Copy Markdown

Congrats! CodSpeed is installed 🎉

🆕 2 new benchmarks were detected.

You will start to see performance impacts in the reports once the benchmarks are run from your default branch.

Detected benchmarks


Open in CodSpeed

@Schamper
Schamper force-pushed the declarative branch 4 times, most recently from 6998c7d to 1178dc0 Compare July 1, 2026 16:51
@yunzheng
yunzheng requested a review from Copilot July 10, 2026 05:28

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.

Pull request overview

This PR introduces a new declarative syntax for defining flow.record record descriptors by subclassing a RecordBase metaclass-driven base, aiming to preserve backwards compatibility while enabling type-hint-friendly record definitions and optional field docstring extraction.

Changes:

  • Add flow.record.declarative.RecordBase + metaclass to generate RecordDescriptor/record behavior from annotated subclasses (plus get_field_info for extracting inline attribute docstrings).
  • Add extensive tests for declarative records (typing/typenames/defaults/inheritance/init-only vars/docstring extraction) and add a benchmark suite comparing classic vs declarative initialization.
  • Update docs and CI/test tooling to accommodate runtime-evaluated annotations and optional benchmark execution.

Reviewed changes

Copilot reviewed 9 out of 9 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
tox.ini Adjust test invocation and add a benchmark tox environment.
tests/test_benchmark.py Add benchmark tests comparing classic vs declarative record construction.
tests/record/test_declarative.py Add functional test coverage for declarative record behavior and docstring extraction.
tests/conftest.py Conditionally register/skip the benchmark marker when the plugin isn’t installed.
README.md Document declarative record definitions and usage patterns (InitVar, __post_init__, Ruff note).
pyproject.toml Add Ruff TC configuration for runtime-evaluated base classes and add typing_extensions to a dependency group.
flow/record/declarative.py Introduce the declarative record implementation (RecordBase, field, get_field_info).
flow/record/base.py Factor out _FIELDTYPES_PREFIX and add _default_is_trivial to optimize default handling.
.github/workflows/dissect-ci.yml Enable running benchmarks in CI via the shared workflow template input.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread tox.ini Outdated
Comment thread flow/record/declarative.py
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.

POC adding dataclass-style declaration to flow.record

2 participants