diff --git a/.jules/bolt.md b/.jules/bolt.md index d54cf10fc..469dd8c61 100644 --- a/.jules/bolt.md +++ b/.jules/bolt.md @@ -61,3 +61,6 @@ ## 2026-07-13 - Array.from mapping optimization **Learning:** Using `Array.from({ length: N }).map(...)` creates an intermediate array of `undefined` values which requires memory allocation and garbage collection, adding O(N) unnecessary overhead in frequently re-rendered UI components. **Action:** Use `Array.from({ length: N }, (_, index) => ...)` to map elements directly during array creation, avoiding intermediate allocations. +## 2024-05-14 - Python O(N^2) list dedup bottleneck +**Learning:** Found O(N^2) deduplication using `if item not in list:` inside nested loops in `services/analysis-engine/src/bandscope_analysis/exports/chart.py`. Given that memory instructions suggest using dictionary keys for deduplication (`dict.fromkeys(items)` or `seen[item] = None`) to maintain O(1) lookups and preserve insertion order, this is a measurable performance anti-pattern. +**Action:** Replace `item not in list` with dict key lookups (`dict.fromkeys()` where applicable, or a `seen` dict) in hot paths like export and formatting routines. diff --git a/services/analysis-engine/src/bandscope_analysis/exports/chart.py b/services/analysis-engine/src/bandscope_analysis/exports/chart.py index 3a84b59c8..6d853bf0b 100644 --- a/services/analysis-engine/src/bandscope_analysis/exports/chart.py +++ b/services/analysis-engine/src/bandscope_analysis/exports/chart.py @@ -79,11 +79,13 @@ def _active_role_ids(section: Mapping[str, object]) -> list[str] | None: if not isinstance(part_graph, list): return None active: list[str] = [] + seen: set[str] = set() for node in part_graph: if not isinstance(node, Mapping) or node.get("is_active") is not True: continue role_id = node.get("role_id") - if isinstance(role_id, str) and role_id and role_id not in active: + if isinstance(role_id, str) and role_id and role_id not in seen: + seen.add(role_id) active.append(role_id) return active @@ -122,9 +124,12 @@ def _role_display_name(role: Mapping[str, object]) -> str | None: def _active_role_names(section: Mapping[str, object]) -> list[str]: """Return de-duplicated display names for the section's active roles.""" names: list[str] = [] + # Use expected O(1) dictionary membership instead of O(N) list membership. + seen: dict[str, None] = {} for role in _active_roles(section): name = _role_display_name(role) - if name is not None and name not in names: + if name is not None and name not in seen: + seen[name] = None names.append(name) return names @@ -132,12 +137,15 @@ def _active_role_names(section: Mapping[str, object]) -> list[str]: def _section_cue(section: Mapping[str, object]) -> str: """Join the active roles' cue values into a single cue string.""" cues: list[str] = [] + # Use expected O(1) dictionary membership instead of O(N) list membership. + seen: dict[str, None] = {} for role in _active_roles(section): cue = role.get("cue") if not isinstance(cue, Mapping): continue value = cue.get("value") - if isinstance(value, str) and value and value not in cues: + if isinstance(value, str) and value and value not in seen: + seen[value] = None cues.append(value) return "; ".join(cues) @@ -189,6 +197,8 @@ def _footer_lines(song: Mapping[str, object], sections: list[Mapping[str, object """Build the footer: per-role rehearsal priorities and the export focus.""" lines: list[str] = [] priorities: list[str] = [] + # Use expected O(1) dictionary membership instead of O(N) list membership. + seen_priorities: dict[str, None] = {} for section in sections: for role in _section_roles(section): name = _role_display_name(role) @@ -196,7 +206,8 @@ def _footer_lines(song: Mapping[str, object], sections: list[Mapping[str, object if name is None or not isinstance(priority, str) or not priority: continue entry = f" - {name}: {priority}" - if entry not in priorities: + if entry not in seen_priorities: + seen_priorities[entry] = None priorities.append(entry) if priorities: lines.append("Priorities:") diff --git a/services/analysis-engine/tests/test_chart_export_dedup_complexity.py b/services/analysis-engine/tests/test_chart_export_dedup_complexity.py new file mode 100644 index 000000000..b47ee1459 --- /dev/null +++ b/services/analysis-engine/tests/test_chart_export_dedup_complexity.py @@ -0,0 +1,37 @@ +"""Deterministic complexity regression for chart-export role de-duplication.""" + +from typing import ClassVar + +from bandscope_analysis.exports.chart import _active_role_ids + + +class _CountingRoleId(str): + """String role id that counts equality work without wall-clock timing.""" + + comparisons: ClassVar[int] = 0 + + def __eq__(self, other: object) -> bool: + """Count one equality comparison and preserve normal string semantics.""" + type(self).comparisons += 1 + return super().__eq__(other) + + def __hash__(self) -> int: + """Preserve normal string hashing for representative hash membership.""" + return super().__hash__() + + +def test_active_role_id_deduplication_avoids_quadratic_equality_work() -> None: + """Unique active ids must not require pairwise list-membership comparisons.""" + role_ids = [_CountingRoleId(f"role-{index}") for index in range(64)] + section = { + "partGraph": [ + {"role_id": role_id, "is_active": True} + for role_id in [*role_ids, _CountingRoleId("role-0")] + ] + } + + _CountingRoleId.comparisons = 0 + active = _active_role_ids(section) + + assert active == role_ids + assert _CountingRoleId.comparisons < 256