You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
{{ message }}
Repository navigation
form: declare configuration names and on-disk tokens independently - #1039
A technology's on-disk token was derived by lowercasing its configuration spelling, tying a persistent file-format identifier to a user-interface string: renaming a configuration name would silently have changed what FORM writes to disk. There was no parser in the other direction either, so the descriptor read back in #1033 could not be turned into a technology::id.
Declare both spellings in one constexpr registry in core/technology.hpp, holding each technology's id, configuration name, on-disk token and whether FORM implements it. Every lookup and both parsers derive from that table.
Add a separate exact-match parsing path for on-disk tokens: on_disk_token(), find_on_disk_token() and from_on_disk_token().
Added a constexpr technology registry with independent configuration names, on-disk tokens, IDs, and implementation status.
Added on_disk_token(), find_on_disk_token(), and from_on_disk_token(). Token parsing uses exact matches and rejects unknown or unimplemented technologies.
Kept configuration parsing separate and exact-match. It now identifies on-disk tokens when reporting configuration-name errors.
Updated physical row-space names, navigation names, and descriptor writing to use explicit on-disk tokens. Removed the derived technology_name() helper.
Tests
Added coverage for canonical token mappings, token round trips, case-variant and unknown-token rejection, unimplemented technologies, and unchanged configuration parsing.
Updated descriptor and navigation tests to expect on-disk tokens.
A technology's on-disk token was derived by lowercasing its configuration
spelling, tying a persistent file-format identifier to a user-interface
string: renaming a configuration name would silently have changed what FORM
writes to disk. There was no parser in the other direction either, so the
descriptor read back in Framework-R-D#1033 could not be turned into a technology::id.
Declare both spellings in one constexpr registry in core/technology.hpp,
holding each technology's id, configuration name, on-disk token and whether
FORM implements it. Every lookup and both parsers derive from that table.
Add a separate exact-match parsing path for on-disk tokens: on_disk_token(),
find_on_disk_token() and from_on_disk_token().
Auto incremental reviews are disabled on this repository.
Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.
Run configuration
Source mode: Auto
Repository: Unavailable
Branch: Unavailable
File: No configuration file from this repository applied
Commit: Unavailable
Layers: Organization UI
Review profile: ASSERTIVE
Plan: Advanced
Run ID: 1119c864-db52-45bf-99ce-e6914de53323
You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.
Use the checkbox below for a quick retry:
Trigger review
No actionable comments were generated in the recent review.
Walkthrough
The technology registry now keeps configuration names separate from persistent on-disk tokens. Naming, persistence output, and navigation checks use the token API, with tests covering parsing, token mappings, and round trips.
Changes: Technology Token Handling
Layer / File(s)
Summary
Register technology names and tokens form/core/technology.hpp, test/form/form_basics_test.cpp
A registry now defines configuration names, on-disk tokens, and implementation status. Parsing and conversion functions use the registry. Tests cover exact configuration-name parsing, token mappings, and invalid or unimplemented technologies.
Use tokens in names and descriptors form/core/container_naming.hpp, form/persistence/navigation_naming.hpp, form/persistence/persistence_writer.cpp, test/form/form_basics_test.cpp, test/form/form_storage_test.cpp
Physical and navigation names, persistence descriptors, and mismatch errors use on-disk tokens. Storage tests check token values and conversions back to technology IDs.
Align navigation checks with tokens test/form/navigation_check.hpp, test/form/dune_example_navigation_dump.cpp, test/form/form_navigation_test.cpp
Navigation discovery and validation derive tokens from technology IDs. Related tests and the navigation dump example use the token API.
Framework-R-D/phlex#1018: Adds the write-path layout descriptor and records technologies in each file; this change updates the descriptor technology representation.
Framework-R-D/phlex#1033: Reads layout descriptors and parses technology values; this change adds a separate parser for on-disk tokens.
The change separates configuration names from persistent tokens while preserving existing spellings. No material merge risk was identified.
Pre-merge checks: 4 · 1
Failed checks: 1 warning
Check name
Status
Explanation
Resolution
Docstring Coverage
Docstring coverage is 58.82% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff with cyclomatic complexity of at least 2. Documenta…
Write docstrings for the functions missing them to satisfy the coverage threshold.
Passed checks: 4 passed
Check name
Status
Explanation
Description Check
Check skipped - CodeRabbit’s high-level summary is enabled.
Title check
The title clearly and concisely describes the main change: separating configuration names from on-disk technology tokens.
Linked Issues check
Objectives from #1034, checked against head 136ed55:
1. Independent declarations [#1034]: PASS. form/core/technology.hpp adds detail::registry, one registration row per technology. Each row hol…
Out of Scope Changes check
Every changed area supports #1034: - form/core/technology.hpp: the registry, the configuration and on-disk token APIs, and the compile-time checks. This is the core requirement. - `form/core/contain…
Full details: Linked Issues check
Explanation
Objectives from #1034, checked against head 136ed55:
Independent declarations [#1034]: PASS. form/core/technology.hpp adds detail::registry, one registration row per technology. Each row holds the id, configuration name, on-disk token, and is_implemented. to_string, from_string, on_disk_token, find_on_disk_token, and from_on_disk_token all read from this table. Compile-time checks reject empty tokens and duplicate ids, tokens, or non-empty configuration names.
Explicit on-disk tokens in writers [#1034]: PASS. technology_name() is removed from form/core/container_naming.hpp. build_row_space_name uses form::technology::on_disk_token(tech). navigation_naming.hpp and persistence_writer.cpp also switch to on_disk_token. The lowercasing and sanitizing path no longer exists.
Exact-match descriptor-token parser, no case folding [#1034]: PASS. from_on_disk_token and find_on_disk_token call lookup_by_on_disk_token, which uses == only. The test checks that ROOT_TTREE, Root_TTree, and GENERIC are rejected.
Preserved tokens root_ttree, root_rntuple, generic [#1034]: PASS. The registry declares these exact spellings. test/form/form_basics_test.cpp ('a navigation name carries the technology that wrote it') now checks on_disk_token against the same three strings.
Configuration parsing unchanged [#1034]: PASS. Base from_string accepts ROOT_TTREE and ROOT_RNTUPLE, and throws for HDF5 as not implemented and for other names as unknown. Head accepts the same set and throws the same way. The only difference is a clarifying suffix in the error text when the input is an on-disk token. to_string returns the same strings as before, and UNKNOWN for generic and unregistered ids. The new test block pins these cases.
Tests [#1034]: PASS. form_basics_test.cpp covers token round trips, rejection of case variants, unknown and empty tokens, unimplemented hdf5, invalid ids, and configuration-name regressions. form_storage_test.cpp and navigation_check.hpp now use the on-disk token.
Residual notes, not failures:
The removed technology_name() callers are identified only through the whole-PR change summary. I could not search the tree for remaining references. The summary lists every changed caller. Any missed caller would fail to compile, so a build should confirm this.
I did not run tests. The PR reports no test results.
The issue lists form/storage/ and form/root_storage/ as affected areas for descriptor consumption. The required-change list does not require a reader change. The new parser exists, but no production reader calls it in this PR. I treat that as outside the stated requirements.
Full details: Out of Scope Changes check
Explanation
Every changed area supports #1034:
form/core/technology.hpp: the registry, the configuration and on-disk token APIs, and the compile-time checks. This is the core requirement.
form/core/container_naming.hpp, form/persistence/navigation_naming.hpp, form/persistence/persistence_writer.cpp: removal of the derived technology_name() path and switch to explicit tokens. Required by the issue.
test/form/form_basics_test.cpp, test/form/form_storage_test.cpp, test/form/form_navigation_test.cpp, test/form/navigation_check.hpp: tests and test helpers that follow the renamed API. The issue requires these tests.
test/form/dune_example_navigation_dump.cpp: the prefix now comes from the parsed id, not the input string. This follows from the removed helper.
No change is unrelated to the issue. One judgment call is noted but not classed as out of scope: the registry declares an on-disk token hdf5 for the unimplemented HDF5 technology, and a test pins that spelling. The issue does not name an HDF5 token. The spelling becomes a persistent identifier once it ships, so the author should confirm it is intended. This sits inside the registry design described in the PR and the issue's independent-declarations requirement.
Full details: Docstring Coverage
Explanation
Docstring coverage is 58.82% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff with cyclomatic complexity of at least 2. Documentation-only edits and deletions within surviving functions are included; unchanged diff context and fully deleted functions are excluded. Analyzed 17 functions across 9 files.
Resolution
Write docstrings for the functions missing them to satisfy the coverage threshold.
Finishing Touches
Generate unit tests (beta)
Create a new PR
Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts
Tip
🎯 Customize your risk assessments
Guide how CodeRabbit assesses merge risk and security architecture risk. Highlight sensitive changes and explain your security boundaries using Merge Risk Instructions and Security Risk Instructions in your review settings. Security architecture risk assessments require the Advanced plan or above.
✅ Automatic approvals, with your rules
Let CodeRabbit approve pull requests once required threads are resolved, pre-merge checks pass, and your risk limits are met. Enable Automatic approvals in review settings, with overrides for teams or file paths.
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.
File: No configuration file from this repository applied
Commit: Unavailable
Layers: Organization UI
Review profile: ASSERTIVE
Plan: Advanced
Run ID: 160e86b5-8a77-4606-96ac-957ec5abf7bc
Commits
Reviewing files that changed from the base of the PR and between ccba655 and 136ed55.
Files selected for processing: 9
form/core/container_naming.hpp
form/core/technology.hpp
form/persistence/navigation_naming.hpp
form/persistence/persistence_writer.cpp
test/form/dune_example_navigation_dump.cpp
test/form/form_basics_test.cpp
test/form/form_navigation_test.cpp
test/form/form_storage_test.cpp
test/form/navigation_check.hpp
Included review availability: This review used your included allowance. Your plan provides up to 1 included review per hour; 0 remain after this review.
Source excerpt: **Authoritative sources**: `.clang-format` and `.clang-tidy` are the ground truth for formatting and naming.
CodeRabbit inference engine (AGENTS.md)
Files:
test/form/dune_example_navigation_dump.cpp
form/persistence/persistence_writer.cpp
test/form/form_storage_test.cpp
form/persistence/navigation_naming.hpp
test/form/form_navigation_test.cpp
test/form/navigation_check.hpp
form/core/container_naming.hpp
test/form/form_basics_test.cpp
form/core/technology.hpp
Learnings: 1
Common learnings
Learnt from: wwuoneway
Repo: Framework-R-D/phlex PR: 1033
File: :0-0
Timestamp: 2026-11-04T00:00:02.106Z
Learning: Within repository "Framework-R-D/phlex", only for files matching one of ["form/core/container_naming.hpp","test/form/form_basics_test.cpp"]: When reviewing changes to technology_name() in form/core/container_naming.hpp or its token expectations in test/form/form_basics_test.cpp, check that explicit on-disk technology token declarations preserve root_ttree, root_rntuple, and generic, as pinned by the section "a navigation name carries the technology that wrote it". Do not change these persistent spellings to follow configuration-name changes.
Additional comments: 9
form/core/technology.hpp: 1
4-5: LGTM!
Also applies to: 7-7, 9-9, 14-20, 45-46, 51-142, 144-146, 148-150, 152-152, 155-156, 158-164, 166-168, 170-170, 173-175, 177-180, 182-182, 185-203
test/form/form_basics_test.cpp: 1
39-40: LGTM!
Also applies to: 46-46, 251-251, 270-319, 1080-1082
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
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.
A technology's on-disk token was derived by lowercasing its configuration spelling, tying a persistent file-format identifier to a user-interface string: renaming a configuration name would silently have changed what FORM writes to disk. There was no parser in the other direction either, so the descriptor read back in #1033 could not be turned into a technology::id.
Declare both spellings in one constexpr registry in core/technology.hpp, holding each technology's id, configuration name, on-disk token and whether FORM implements it. Every lookup and both parsers derive from that table.
Add a separate exact-match parsing path for on-disk tokens: on_disk_token(), find_on_disk_token() and from_on_disk_token().
This is about to close #1034.
Code
on_disk_token(),find_on_disk_token(), andfrom_on_disk_token(). Token parsing uses exact matches and rejects unknown or unimplemented technologies.technology_name()helper.Tests