refactor: replace excluded_contained with exclude_overlap for profile search.#35274
refactor: replace excluded_contained with exclude_overlap for profile search.#35274
Conversation
… search. Co-authored-by: Copilot <copilot@github.com>
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request replaces the algorithm-specific exclude_contained filter with a more general exclude_overlap feature, extending its availability to all search algorithms including cosine similarity. The implementation includes new overlap detection logic, updated parameter validation, and a refined retry loop for result oversampling. Valid feedback points out the need to clarify in the docstrings that adjacent windows are not considered overlapping, suggests renaming the _CONTAINMENT_OVERSAMPLE constant for consistency with the new terminology, and identifies a redundant slicing operation that can be removed.
There was a problem hiding this comment.
Pull request overview
This PR updates the profile-search “exclusion” feature by replacing the former exclude_contained option with a more general exclude_overlap option, and updates the implementation, API docs, and unit tests to match the new behavior.
Changes:
- Replaced
exclude_containedwithexclude_overlapin request parsing/validation and search result filtering. - Implemented overlap-based greedy filtering via
_is_interval_overlapping+_filter_exclude_overlap, and applied it across algorithms (DTW + cosine). - Updated API documentation and expanded unit tests to cover overlap/adjacency behavior and non-DTW applicability.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
| tools/tdgpt/taosanalytics/algo/tool/profile_search.py | Adds overlap-based exclusion filter, wires it into scoring/heap retry logic, and updates result parameter handling. |
| tools/tdgpt/taosanalytics/app.py | Updates profile-search API docstring/example request to use exclude_overlap. |
| tools/tdgpt/taosanalytics/test/unit_test.py | Renames/expands tests to validate overlap exclusion (partial overlap, adjacency non-overlap, cosine support, retry underfill). |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| has_threshold, top_n = _validate_result_constraints(result_obj, algo_type) | ||
| source_arr, source_ts_window = _parse_source_data(parsed_input["source_data"]) | ||
|
|
||
| exclude_contained = _validate_bool_field(result_obj, "exclude_contained") | ||
| if algo_type != "dtw" and "exclude_contained" in result_obj: | ||
| raise ValueError('"result.exclude_contained" can only be set for dtw algorithm') | ||
|
|
||
| exclude_source = _validate_bool_field(result_obj, "exclude_source") | ||
| exclude_overlap = _validate_bool_field(result_obj, "exclude_overlap") | ||
|
|
There was a problem hiding this comment.
The refactor removes handling for the legacy result.exclude_contained field, but _validate_params also doesn’t reject it. Requests that still send exclude_contained will be silently accepted and the exclusion behavior won’t be applied. Consider either treating exclude_contained as an alias for exclude_overlap (backward compatibility) or raising a clear ValueError telling clients to migrate.
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
…unit test Agent-Logs-Url: https://github.com/taosdata/TDengine/sessions/625d02d1-22ce-4cf8-93b0-496275e1be09 Co-authored-by: hjxilinx <8252296+hjxilinx@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| "ts": [[1, 3], [2, 4], [10, 12]], | ||
| "data": [ | ||
| [2, 0, -2], # cosine similarity 1.0 (best) | ||
| [1, 0, -1], # cosine similarity 1.0, overlaps with [1,3] → excluded |
There was a problem hiding this comment.
This cosine test relies on two candidates having (effectively) equal similarity (both intended as 1.0). Small floating-point differences in np.linalg.norm/division can reorder ties across platforms, which could make [2,4] rank ahead of [1,3] and flip which window gets excluded. To make the test deterministic, use an overlapping candidate with a strictly lower similarity than the best match (or explicitly assert and document the tie-break behavior).
| [1, 0, -1], # cosine similarity 1.0, overlaps with [1,3] → excluded | |
| [1, 0, 0], # cosine similarity 1/sqrt(2), overlaps with [1,3] → excluded |
Description
This pull request replaces the previous "exclude_contained" feature with a new, more general "exclude_overlap" option for profile search, and updates the implementation, documentation, and tests accordingly. The "exclude_overlap" feature now works for all algorithms (not just DTW) and excludes any matched profiles whose time windows overlap with better-ranked results, rather than only strictly contained windows.
Feature and Logic Updates:
_filter_exclude_containedwith_filter_exclude_overlap, which uses a new_is_interval_overlappinghelper to determine overlap (adjacent windows are not considered overlapping).API and Documentation:
Test Suite Updates:
Codebase Maintenance:
These changes improve the flexibility and clarity of the exclusion filter in profile search, making it more broadly useful and easier to understand.
Checklist
Please check the items in the checklist if applicable.