Skip to content

fix(SyntheticTimeSeries): Make synthetic time series more robust with special characters - #2766

Open
MortGron wants to merge 7 commits into
masterfrom
synthetic-time-series-variables-with-dash
Open

fix(SyntheticTimeSeries): Make synthetic time series more robust with special characters#2766
MortGron wants to merge 7 commits into
masterfrom
synthetic-time-series-variables-with-dash

Conversation

@MortGron

Copy link
Copy Markdown
Contributor

Description

By mistake, synthetic time series queries will fail when re.escape(var) != var for some variable var in a query expression before variable replacement. However, they do not fail when variables contain characters like \.

Even though this fixes a bug, a long term solution to make variable names safer is #2762

Checklist:

  • Tests added/updated.
  • Documentation updated. Documentation is generated from docstrings - these must be updated according to your change.
    If a new method has been added it should be referenced in cognite.rst in order to generate docs based on its docstring.
  • The PR title follows the Conventional Commit spec.

@codecov

codecov Bot commented Aug 13, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 93.08%. Comparing base (51d717d) to head (e2cb6e8).

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #2766      +/-   ##
==========================================
- Coverage   93.09%   93.08%   -0.02%     
==========================================
  Files         516      516              
  Lines       53590    53597       +7     
==========================================
+ Hits        49889    49890       +1     
- Misses       3701     3707       +6     
Files with missing lines Coverage Δ
cognite/client/_api/synthetic_time_series.py 96.00% <100.00%> (+0.03%) ⬆️
cognite/client/_sync_api/synthetic_time_series.py 100.00% <ø> (ø)
.../tests_unit/test_api/test_synthetic_time_series.py 97.70% <100.00%> (+0.10%) ⬆️

... and 6 files with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@MortGron
MortGron marked this pull request as ready for review August 31, 2026 12:57
@MortGron
MortGron requested review from a team as code owners August 31, 2026 12:57

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Code Review

This pull request updates the synthetic time series expression builder to escape variable names during regex compilation rather than when storing them in the substitution dictionary, and updates the unit tests accordingly. The reviewer suggests sorting the substitution keys by length in descending order before compiling the regex pattern to prevent shorter prefixes from matching first when they contain non-word characters.

Comment thread cognite/client/_api/synthetic_time_series.py
@MortGron
MortGron requested a review from haakonvt September 2, 2026 14:24

@haakonvt haakonvt left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🦄 Preferably add test cases in a parametrize, but not a hard requirement

@haakonvt haakonvt changed the title fix(SyntheticTimeSeries): synthetic time series query fails when variable contains certain characters fix(SyntheticTimeSeries): Make synthetic time series more robust with special characters Sep 3, 2026
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.

2 participants