fix(context): dict_parser reads aliases key for dimension/metric synonyms - #29
Open
Zyl0812 wants to merge 1 commit into
Open
fix(context): dict_parser reads aliases key for dimension/metric synonyms#29Zyl0812 wants to merge 1 commit into
Zyl0812 wants to merge 1 commit into
Conversation
SemanticImportRequest's DimensionPayload/MetricPayload only define an
`aliases` field (see contracts/import_models.py); there is no `synonyms`
field on the wire. dict_parser._convert_dimensions/_convert_metrics read
`dim.get("synonyms")`/`met.get("synonyms")`, which is always None for
payloads produced by weave_assembler, so every alias imported via the
Excel semantic-config flow was silently dropped during weave (weave
still returns SUCCESS, no error surfaced).
Read `aliases` first, falling back to `synonyms` for the legacy YAML
import path -- matching the pattern already used for domain aliases
(dict_parser.py:58) and dimension value aliases (dict_parser.py:346).
Add regression tests covering both keys for _convert_dimensions and
_convert_metrics.
Signed-off-by: zhouyilong <zhouyilong@mysteel.com>
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
1 participant
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.
What
dict_parser._convert_dimensionsand_convert_metricsread asynonymskey that doesn't exist on the wire contract, so everydimension/metric alias imported through the Excel semantic-config
flow (
POST /api/semantic-config/import/excel) is silently droppedwhen weave runs -- the graph ends up with empty
synonymsarrays,and weave still reports
SUCCESS.Why
SemanticImportRequest'sDimensionPayload/MetricPayload(
contracts/import_models.py) only define analiasesfield.weave_assembler.pyproduces payloads keyed onaliasesaccordingly. But
dict_parser.py:312and:416readdim.get("synonyms")/met.get("synonyms"), which is alwaysNonefor these payloads -- the two spots weren't updated when thedomain-alias (
dict_parser.py:58) and dimension-value-alias(
dict_parser.py:346) conversions were already fixed to readaliases.Found while importing ~1,800 real dimension/metric definitions
through the Excel flow:
metric.synonymsanddimension.synonymswere
[]for every node after weave, despite the source Excelhaving non-empty
synonymscolumns for ~1,400 of them.Fix
Read
aliasesfirst, fall back tosynonyms(keeps the legacyYAML-based import path working, since that path already uses
synonymsas the key name).How verified
tests/test_dict_parser_synonyms.py: 4 cases covering_convert_dimensions/_convert_metricsreadingaliases, andfalling back to
synonyms."reads aliases" cases fail with an empty list) and pass after
the fix.
uv run python -m compileall packagesanduv run pytest -q packages/datapaw-context/tests(152 passed)both clean.