Repository navigation
fix(cpp): emit bare measurement names for tree-model result columns - #993
Merged
ColinLeeo merged 2 commits intoOct 9, 2026
Merged
Conversation
Tree-model head/cat/export emitted fully-qualified "device.measurement"
column names (and NDJSON keys), but the CLI requirement specifies bare
measurement names: e.g. `head -d root.factory.d1 -m temp -m status`
should print `time temp status`, and `head -m temp -f ndjson` should
emit {"time":"...","temp":...}.
Strip the device prefix at the output layer only for tree-model row
queries, leaving the reader's get_column_name contract and the table
model untouched.
Contributor
There was a problem hiding this comment.
Warning
Copilot couldn't run its full agentic review because it didn't start before the timeout. Make sure your repository has a runner available, or add a copilot-code-review.yml file specifying one with the runs-on attribute. See the docs for more details.
Copilot review overview
4 open findings
time,… · Newtime,… · Newconst std::string&(= \"\") forces construction of a temporary… · Newstrip_device_prefixsuggests the caller passes a full prefix (possibly… · New
What changed in this PR
Adjusts CLI output for tree-model row queries so result columns (headers and NDJSON keys) use bare measurement names instead of fully-qualified device.measurement, matching the CLI requirement examples.
Changes:
- Adds an optional device-prefix stripping parameter to
emit_result_setand applies it in the tree-modelrow_querycommand path. - Updates formatting logic to strip
"<device>."from column names at output time. - Updates/extends CLI e2e + golden outputs to assert bare measurement names for CSV/table/NDJSON.
| File | Description |
|---|---|
| cpp/tools/format/result_set_format.h | Extends emit_result_set API with optional device-prefix stripping parameter. |
| cpp/tools/format/result_set_format.cc | Implements prefix stripping when building output headers. |
| cpp/tools/commands/row_query.cc | Passes resolved tree device into formatter to enable stripping for tree-model queries only. |
| cpp/test/tools/model_format_e2e_test.cc | Updates expected CSV header to bare measurement names. |
| cpp/test/tools/command_e2e_test.cc | Adds coverage for implicit device resolution still producing bare measurement output. |
| cpp/test/tools/golden/tree_* | Updates golden outputs (table/csv/ndjson) to bare measurement names. |
🧠 Review effort: Lite
Give feedback about Copilot approvals in this survey to enter a drawing for a $150 gift card.
Comment on lines
+81
to
+82
| const std::string device_prefix = | ||
| strip_device_prefix.empty() ? std::string() : strip_device_prefix + "."; |
Comment on lines
+88
to
+93
| std::string name = meta->get_column_name(i); | ||
| if (!device_prefix.empty() && name.size() > device_prefix.size() && | ||
| name.compare(0, device_prefix.size(), device_prefix) == 0) { | ||
| name = name.substr(device_prefix.size()); | ||
| } | ||
| header.push_back(std::move(name)); |
Comment on lines
+38
to
39
| const std::string& strip_device_prefix = ""); | ||
|
|
Comment on lines
+81
to
+82
| const std::string device_prefix = | ||
| strip_device_prefix.empty() ? std::string() : strip_device_prefix + "."; |
Copilot stopped reviewing on behalf of
ColinLeeo due to an error
October 8, 2026 09:09
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Tree-model head/cat/export emitted fully-qualified "device.measurement" column names (and NDJSON keys), but the CLI requirement specifies bare measurement names: e.g.
head -d root.factory.d1 -m temp -m statusshould printtime temp status, andhead -m temp -f ndjsonshould emit {"time":"...","temp":...}.Strip the device prefix at the output layer only for tree-model row queries, leaving the reader's get_column_name contract and the table model untouched.