diff --git a/.jules/bolt.md b/.jules/bolt.md index d54cf10fc..7b0f5cee8 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. +## 2026-03-06 - [파이썬 O(N^2) 리스트 룩업을 O(1) 딕셔너리로 최적화] +**Learning:** `chart.py`의 텍스트 변환 로직에서 `not in list`로 중복을 방지하며 삽입하는 방식은 리스트 크기가 커질 때 O(N^2) 병목을 유발합니다. 파이썬 3.7+부터 딕셔너리가 삽입 순서를 유지하므로, `dict[item] = None`을 사용해 순서를 보존하면서 O(1)의 성능 최적화가 가능함을 배웠습니다. +**Action:** 앞으로 리스트의 중복을 제거하면서 순서를 유지해야 하는 로직에서는 `set` 대신 딕셔너리(dictionary) 키를 활용할 것입니다. diff --git a/CHANGELOG.md b/CHANGELOG.md index 34331fb86..f66f16d03 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -10,6 +10,7 @@ ### Changed +- Changed chart-export role, cue, and priority de-duplication to insertion-ordered dictionaries, preserving first-occurrence output while replacing repeated linear membership scans with average constant-time key lookups. - Consolidated Bandit, dependency audits, supplemental secret checks, and Trivy into one trusted-branch security backstop, delegated CodeQL to GitHub default setup, and removed duplicate local PR security and release-preflight runs. - Pinned npm `10.9.9` as the approved lockfile generator, activated it through Node-bundled Corepack before dependency consumption, and fail closed unless its bundled `tar` is at least `7.5.19`; primary CI still consumes the committed lock only through frozen `npm ci` validation, rejects mutable npm resolution in the lock gate, requires integrity evidence for public-registry lock entries, and preserves generator-sensitive root `@esbuild/*` peer metadata. diff --git a/services/analysis-engine/src/bandscope_analysis/exports/chart.py b/services/analysis-engine/src/bandscope_analysis/exports/chart.py index 3a84b59c8..87651f521 100644 --- a/services/analysis-engine/src/bandscope_analysis/exports/chart.py +++ b/services/analysis-engine/src/bandscope_analysis/exports/chart.py @@ -7,7 +7,7 @@ Security Notes: - Pure dict-to-string transformation: no file, network, or process I/O. - Never reads source-path fields and never emits filesystem paths. - - Safe failure: ``None``, empty, or malformed input yields ``""`` / ``[]``; + - Safe failure: ``None``, empty, or malformed input yields ``\"\"`` / ``[]``; missing or malformed keys are skipped and no exceptions escape. """ @@ -73,19 +73,31 @@ def _section_roles(section: Mapping[str, object]) -> list[Mapping[str, object]]: return [role for role in roles if isinstance(role, Mapping)] +def _hashable_text(value: object) -> str | None: + """Return compatible string-like text as a safe built-in mapping key.""" + if not isinstance(value, str): + return None + try: + hash(value) + text = str.__str__(value) + except Exception: + return None + return text if text else None + + def _active_role_ids(section: Mapping[str, object]) -> list[str] | None: """Return active role ids from the part graph, or ``None`` when absent.""" 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 + role_id = _hashable_text(node.get("role_id")) + if role_id is not None: + active[role_id] = None + return list(active) def _active_roles(section: Mapping[str, object]) -> list[Mapping[str, object]]: @@ -102,43 +114,40 @@ def _active_roles(section: Mapping[str, object]) -> list[Mapping[str, object]]: return roles by_id: dict[str, Mapping[str, object]] = {} for role in roles: - role_id = role.get("id") - if isinstance(role_id, str) and role_id not in by_id: + role_id = _hashable_text(role.get("id")) + if role_id is not None and role_id not in by_id: by_id[role_id] = role return [by_id.get(role_id, {"id": role_id, "name": role_id}) for role_id in active_ids] def _role_display_name(role: Mapping[str, object]) -> str | None: - """Return the role's display name, falling back to its id.""" - name = role.get("name") - if isinstance(name, str) and name: + """Return a hashable display name, falling back to a hashable role id.""" + name = _hashable_text(role.get("name")) + if name is not None: return name - role_id = role.get("id") - if isinstance(role_id, str) and role_id: - return role_id - return None + return _hashable_text(role.get("id")) 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) 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) + value = _hashable_text(cue.get("value")) + if value is not None: + cues[value] = None return "; ".join(cues) @@ -188,16 +197,15 @@ 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) - priority = role.get("rehearsalPriority") - if name is None or not isinstance(priority, str) or not priority: + priority = _hashable_text(role.get("rehearsalPriority")) + if name is None or priority is None: continue entry = f" - {name}: {priority}" - if entry not in priorities: - priorities.append(entry) + priorities[entry] = None if priorities: lines.append("Priorities:") lines.extend(priorities) @@ -216,7 +224,7 @@ def build_chart_text(song: Mapping[str, object] | None) -> str: section (``[mm:ss-mm:ss] LABEL (confidence) roles: ...``), and a footer with rehearsal priorities and the export focus headline. Output is deterministic and never contains filesystem paths. Malformed input - yields ``""``. + yields ``\"\"``. """ if not isinstance(song, Mapping): return "" diff --git a/services/analysis-engine/tests/test_chart_export_dedup.py b/services/analysis-engine/tests/test_chart_export_dedup.py new file mode 100644 index 000000000..0e35b0ca0 --- /dev/null +++ b/services/analysis-engine/tests/test_chart_export_dedup.py @@ -0,0 +1,208 @@ +"""Regression tests for order-preserving chart export de-duplication.""" + +from typing import Any + +from bandscope_analysis.exports import build_chart_text, build_cue_sheet_rows + + +class _UnhashableText(str): + """String-like malformed payload value that cannot be a mapping key.""" + + __hash__: Any = None + + +class _HashableText(str): + """Compatible string subclass that remains safe as a mapping key.""" + + +class _ExplodingTruthText(str): + """Hashable string-like payload whose custom truth check must never run.""" + + def __bool__(self) -> bool: + """Raise if production accidentally delegates truthiness to the subclass.""" + raise TypeError("subclass truthiness must not execute") + + +def _role(role_id: str, name: str, cue: str, priority: str = "") -> dict[str, Any]: + """Build the minimal role evidence consumed by the chart export boundary.""" + return { + "id": role_id, + "name": name, + "cue": {"kind": "entrance", "value": cue}, + "rehearsalPriority": priority, + } + + +def _section( + section_id: str, + label: str, + start: int, + end: int, + roles: list[dict[str, Any]], +) -> dict[str, Any]: + """Build a valid section whose part graph activates roles in list order.""" + part_graph = [{"role_id": role["id"], "is_active": True} for role in roles] + return { + "id": section_id, + "label": label, + "timeRange": {"start": start, "end": end}, + "roles": roles, + "partGraph": part_graph, + } + + +def test_duplicate_display_names_and_cues_keep_first_occurrence_order() -> None: + """Distinct role ids may share display/cue text without duplicating export output.""" + section = _section( + "verse", + "verse", + 0, + 16, + [ + _role("guitar-left", "Guitar", "Count in"), + _role("guitar-right", "Guitar", "Count in"), + _role("bass", "Bass", "Hold root"), + _role("guitar-double", "Guitar", "Count in"), + ], + ) + + rows = build_cue_sheet_rows({"sections": [section]}) + + assert rows == [ + { + "section": "verse", + "start": "00:00", + "end": "00:16", + "cue": "Count in; Hold root", + "roles": ["Guitar", "Bass"], + } + ] + + +def test_duplicate_priorities_across_sections_keep_first_occurrence_order() -> None: + """Repeated name/priority entries collapse once without reordering later entries.""" + song: dict[str, Any] = { + "title": "Order regression", + "sections": [ + _section( + "verse", + "verse", + 0, + 16, + [ + _role("guitar", "Guitar", "Count in", "Lock chorus"), + _role("bass", "Bass", "Hold root", "Watch cutoff"), + ], + ), + _section( + "chorus", + "chorus", + 16, + 32, + [ + _role("guitar-2", "Guitar", "Count in", "Lock chorus"), + _role("bass-2", "Bass", "Hold root", "Watch cutoff"), + ], + ), + ], + } + + text = build_chart_text(song) + priority_lines = text.split("Priorities:\n", maxsplit=1)[1].splitlines() + + assert priority_lines == [ + " - Guitar: Lock chorus", + " - Bass: Watch cutoff", + ] + + +def test_unhashable_string_subclasses_fail_closed_in_public_exports() -> None: + """Malformed unhashable text is skipped while a valid role id remains usable.""" + section = _section( + "verse", + "verse", + 0, + 16, + [ + _role(_UnhashableText("bad-id"), "Bad id", "Bad id cue"), + _role("guitar", _UnhashableText("Guitar"), _UnhashableText("Count in")), + _role("bass", "Bass", "Hold root"), + ], + ) + song = {"sections": [section]} + + assert build_cue_sheet_rows(song) == [ + { + "section": "verse", + "start": "00:00", + "end": "00:16", + "cue": "Hold root", + "roles": ["guitar", "Bass"], + } + ] + assert "roles: guitar, Bass" in build_chart_text(song) + + +def test_hashable_string_subclasses_remain_compatible_export_values() -> None: + """Hashable string subclasses retain pre-optimization role and cue semantics.""" + section = _section( + "verse", + "verse", + 0, + 16, + [ + _role(_HashableText("guitar"), _HashableText("Guitar"), _HashableText("Count in")), + _role("bass", "Bass", "Hold root"), + ], + ) + + assert build_cue_sheet_rows({"sections": [section]}) == [ + { + "section": "verse", + "start": "00:00", + "end": "00:16", + "cue": "Count in; Hold root", + "roles": ["Guitar", "Bass"], + } + ] + + +def test_string_subclass_truthiness_cannot_abort_public_exports() -> None: + """Hashable text is normalized without invoking subclass-defined truthiness.""" + section = _section( + "verse", + "verse", + 0, + 16, + [ + _role("guitar", _ExplodingTruthText("Guitar"), _ExplodingTruthText("Count in")), + _role("bass", "Bass", "Hold root"), + ], + ) + song = {"sections": [section]} + + assert build_cue_sheet_rows(song) == [ + { + "section": "verse", + "start": "00:00", + "end": "00:16", + "cue": "Count in; Hold root", + "roles": ["Guitar", "Bass"], + } + ] + assert "roles: Guitar, Bass" in build_chart_text(song) + + +def test_priority_truthiness_cannot_abort_chart_export() -> None: + """Rehearsal priority text is normalized before footer truthiness checks.""" + section = _section( + "verse", + "verse", + 0, + 16, + [_role("guitar", "Guitar", "Count in", _ExplodingTruthText("Lock chorus"))], + ) + + text = build_chart_text({"sections": [section]}) + + assert " - Guitar: Lock chorus" in 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..89decd0c6 --- /dev/null +++ b/services/analysis-engine/tests/test_chart_export_dedup_contract.py @@ -0,0 +1,90 @@ +"""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"] + + +def test_duplicate_role_ids_preserve_first_payload_and_graph_position() -> None: + """Repeated role identities keep the first role payload and one active position.""" + song: dict[str, Any] = { + "sections": [ + { + "id": "section-1", + "label": "verse", + "timeRange": {"start": 0, "end": 16}, + "roles": [ + _role("bass", "Bass", "Walk up", "high"), + _role("bass", "Bass Copy", "Late replacement", "low"), + ], + "partGraph": [ + {"role_id": "bass", "is_active": True}, + {"role_id": "bass", "is_active": True}, + ], + } + ] + } + + rows = build_cue_sheet_rows(song) + assert rows == [ + { + "section": "verse", + "start": "00:00", + "end": "00:16", + "cue": "Walk up", + "roles": ["Bass"], + } + ]