diff --git a/.jules/bolt.md b/.jules/bolt.md index d54cf10fc..36a066fa0 100644 --- a/.jules/bolt.md +++ b/.jules/bolt.md @@ -61,3 +61,7 @@ ## 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. + +## 2026-08-27 - O(1) dictionary key deduplication for preserving insertion order +**Learning:** Using `if item not in list: list.append(item)` creates an O(N^2) time complexity because `not in list` does a linear scan over the array for every element. +**Action:** Replace `if item not in list: list.append(item)` with O(1) dictionary assignments (`dict[item] = None`) to deduplicate elements in O(N) time while still preserving insertion order (as Python 3.7+ dictionaries maintain insertion order). Use `list(dict.keys())` when returning the final list. diff --git a/services/analysis-engine/src/bandscope_analysis/exports/chart.py b/services/analysis-engine/src/bandscope_analysis/exports/chart.py index 3a84b59c8..9a2a6da40 100644 --- a/services/analysis-engine/src/bandscope_analysis/exports/chart.py +++ b/services/analysis-engine/src/bandscope_analysis/exports/chart.py @@ -78,14 +78,14 @@ def _active_role_ids(section: Mapping[str, object]) -> list[str] | None: part_graph = section.get("partGraph") if not isinstance(part_graph, list): return None - active: list[str] = [] + active: dict[str, None] = {} 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: - active.append(role_id) - return active + if isinstance(role_id, str) and role_id: + active[role_id] = None + return list(active.keys()) def _active_roles(section: Mapping[str, object]) -> list[Mapping[str, object]]: @@ -121,25 +121,25 @@ 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] = [] + names: dict[str, None] = {} for role in _active_roles(section): name = _role_display_name(role) - if name is not None and name not in names: - names.append(name) - return names + if name is not None: + names[name] = None + return list(names.keys()) def _section_cue(section: Mapping[str, object]) -> str: """Join the active roles' cue values into a single cue string.""" - cues: list[str] = [] + cues: 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: - cues.append(value) - return "; ".join(cues) + if isinstance(value, str) and value: + cues[value] = None + return "; ".join(cues.keys()) def _confidence_level(section: Mapping[str, object]) -> str | None: @@ -188,7 +188,7 @@ def _section_lines(sections: list[Mapping[str, object]]) -> list[str]: def _footer_lines(song: Mapping[str, object], sections: list[Mapping[str, object]]) -> list[str]: """Build the footer: per-role rehearsal priorities and the export focus.""" lines: list[str] = [] - priorities: list[str] = [] + priorities: dict[str, None] = {} for section in sections: for role in _section_roles(section): name = _role_display_name(role) @@ -196,11 +196,10 @@ 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: - priorities.append(entry) + priorities[entry] = None if priorities: lines.append("Priorities:") - lines.extend(priorities) + lines.extend(priorities.keys()) summary = song.get("exportSummary") if isinstance(summary, Mapping): headline = summary.get("headline") diff --git a/services/analysis-engine/tests/test_api_path_logging_privacy.py b/services/analysis-engine/tests/test_api_path_logging_privacy.py new file mode 100644 index 000000000..7bba80f99 --- /dev/null +++ b/services/analysis-engine/tests/test_api_path_logging_privacy.py @@ -0,0 +1,50 @@ +"""Privacy regressions for analysis-request rejection logs.""" + +import logging + +import pytest + +from bandscope_analysis.api import validate_analysis_job_request + +_SECRET_MARKER = "private-super-secret" + + +def _local_audio_payload() -> dict[str, object]: + """Build the smallest valid local-audio request used by log-privacy tests.""" + return { + "sourceKind": "local_audio", + "projectId": "project-1", + "sourceLabel": "late-night-set.wav", + "roleFocus": [], + "localSource": { + "sourcePath": "/Users/test/Music/late-night-set.wav", + "fileName": "late-night-set.wav", + "extension": "wav", + "fileSizeBytes": 1_024_000, + }, + } + + +@pytest.mark.parametrize( + ("field", "value"), + [ + ("projectId", f"../{_SECRET_MARKER}"), + ("cacheRoot", f"/tmp/{_SECRET_MARKER}/../cache"), + ("tempRoot", f"C:\\{_SECRET_MARKER}\\..\\Temp"), + ], +) +def test_path_traversal_logs_do_not_echo_untrusted_values( + caplog: pytest.LogCaptureFixture, + field: str, + value: str, +) -> None: + """Rejected path authority logs the operation without user-controlled path text.""" + payload = _local_audio_payload() + payload[field] = value + + with caplog.at_level(logging.WARNING, logger="bandscope_analysis.api"): + with pytest.raises(ValueError, match="path traversal"): + validate_analysis_job_request(payload) + + assert _SECRET_MARKER not in caplog.text + assert "path traversal" in caplog.text diff --git a/services/analysis-engine/tests/test_chart_export_dedup_contract.py b/services/analysis-engine/tests/test_chart_export_dedup_contract.py new file mode 100644 index 000000000..f2fbdda76 --- /dev/null +++ b/services/analysis-engine/tests/test_chart_export_dedup_contract.py @@ -0,0 +1,58 @@ +"""Regression contract for ordered chart-export de-duplication.""" + +from typing import Any + +from bandscope_analysis.exports import build_chart_text, build_cue_sheet_rows + + +def _role(role_id: str, name: str, cue: str, priority: str) -> dict[str, Any]: + """Build the minimum role shape consumed by the chart exporter.""" + return { + "id": role_id, + "name": name, + "cue": {"kind": "entrance", "value": cue}, + "rehearsalPriority": priority, + } + + +def _song() -> dict[str, Any]: + """Build ordered duplicate values that must keep first-occurrence order.""" + return { + "title": "Ordered Dedup Contract", + "sections": [ + { + "id": "section-1", + "label": "verse", + "timeRange": {"start": 0, "end": 16}, + "roles": [ + _role("bass-main", "Bass", "Walk up", "high"), + _role("drums", "Drums", "Hit on 1", "medium"), + _role("bass-copy", "Bass", "Walk up", "high"), + ], + "partGraph": [ + {"role_id": "bass-main", "is_active": True}, + {"role_id": "drums", "is_active": True}, + {"role_id": "bass-main", "is_active": True}, + {"role_id": "bass-copy", "is_active": True}, + ], + } + ], + } + + +def test_ordered_deduplication_preserves_first_occurrence_semantics() -> None: + """Duplicate ids and display values collapse without reordering the chart.""" + rows = build_cue_sheet_rows(_song()) + assert rows == [ + { + "section": "verse", + "start": "00:00", + "end": "00:16", + "cue": "Walk up; Hit on 1", + "roles": ["Bass", "Drums"], + } + ] + + text = build_chart_text(_song()) + priority_lines = [line for line in text.splitlines() if line.startswith(" - ")] + assert priority_lines == [" - Bass: high", " - Drums: medium"]