Files
cowork-local/tests/ui/test_co4e_sidebar_skill_index.py
T

175 lines
7.7 KiB
Python

"""Co4E left column: the skill library must be read ONCE per sidebar reload.
Why this test exists
--------------------
``Co4ESidebarMixin._reload_sidebar`` used to call
``core.skills.skill_prefix_for(name)`` once per skill. Every one of those calls
re-reads the *whole* skill folder from disk (``list_skills()`` +
``builtin_skills()``), so the reload was O(N**2) in the number of skills.
That reload runs on every language switch (``Co4ETab._retranslate`` ->
``_reload_sidebar``). Measured end-to-end on a real library of 121 skills, the
per-name calls cost ~3.8 s of blocked GUI thread — the "app freezes for a few
seconds when I switch Vietnamese -> English" the user reported.
Two different things are guarded here, and they fail for different reasons:
* ``test_reload_sidebar_reads_the_skill_library_once`` — the performance
contract. Red before the fix (the library was read 1 + N times), green after.
* the two equivalence tests — the *correctness* contract. A lookup table is
easy to build subtly wrong, and a wrong one silently changes which skill text
is pushed into an agent prompt. They pin the answer to ``skill_prefix_for``
itself in exactly the cases where a naive ``{name: skill}`` dict diverges:
a name shared by a user skill and a built-in (first match wins, user first),
a skill whose instructions are blank (it does NOT end ``skill_prefix_for``'s
scan, so a later namesake with real instructions must still win), and
lookups by slug / different case / padding / unknown name / empty name.
Those two are characterization tests: green before AND after by design.
"""
from __future__ import annotations
from dataclasses import replace
import pytest
from PySide6.QtCore import Qt
from PySide6.QtWidgets import QListWidget
from cowork_local.core import co4e, skills as skills_mod
from cowork_local.presentation.co4e.co4e_sidebar import Co4ESidebarMixin
class _SidebarHarness(Co4ESidebarMixin):
"""Just enough state to run the real ``_reload_sidebar``.
Building a whole ``Co4ETab`` would drag in config/HOME-dependent module
constants (see ``tests/test_build_co4e_tab.py``); the mixin only touches
the three list widgets, so the real production method runs unchanged here.
"""
def __init__(self) -> None:
"""Create the three palette lists ``_reload_sidebar`` fills in."""
self.wf_list = QListWidget()
self.agent_list = QListWidget()
self.skill_list = QListWidget()
@pytest.fixture
def harness(qapp, monkeypatch):
"""A sidebar harness with the workflow/agent sources stubbed out empty."""
monkeypatch.setattr(co4e, "list_workflows", lambda: [])
monkeypatch.setattr(co4e, "list_custom_agents", lambda: [])
return _SidebarHarness()
def _install_skills(monkeypatch, user, builtin):
"""Replace the two disk-reading skill sources and count how often they run.
Fresh copies are handed out on every call so a caller mutating a returned
``Skill`` cannot make a later call look different.
"""
calls = {"list_skills": 0, "builtin_skills": 0}
def _list_skills(directory=None):
"""Stand-in for ``skills.list_skills`` that records each read."""
calls["list_skills"] += 1
return [replace(s) for s in user]
def _builtin_skills():
"""Stand-in for ``skills.builtin_skills`` that records each read."""
calls["builtin_skills"] += 1
return [replace(s) for s in builtin]
monkeypatch.setattr(skills_mod, "list_skills", _list_skills)
monkeypatch.setattr(skills_mod, "builtin_skills", _builtin_skills)
return calls
def _skill_rows(sidebar):
"""Return ``(item text, payload instructions)`` for every skill palette row."""
return [(sidebar.skill_list.item(i).text(),
sidebar.skill_list.item(i).data(Qt.UserRole)["instructions"])
for i in range(sidebar.skill_list.count())]
def test_reload_sidebar_reads_the_skill_library_once(harness, monkeypatch):
"""One sidebar reload must hit the skill library exactly once, not once per skill."""
user = [skills_mod.Skill(name=f"Skill {i}", instructions=f"Body {i}")
for i in range(8)]
calls = _install_skills(monkeypatch, user, [])
harness._reload_sidebar()
assert harness.skill_list.count() == 8, "every skill must still reach the palette"
assert calls["list_skills"] == 1, (
f"the skill library was read {calls['list_skills']}x for 8 skills — "
"reading it once per skill is the O(N^2) freeze on language switch"
)
assert calls["builtin_skills"] == 1, (
f"built-in skills were read {calls['builtin_skills']}x for 8 skills"
)
def test_skill_payloads_match_skill_prefix_for(harness, monkeypatch):
"""The palette payload must be byte-identical to ``skill_prefix_for``'s answer.
The fixture is deliberately hostile: a name shared by a user skill and a
built-in, a blank-instructions skill followed by a namesake with real
content, and names with padding/odd casing.
"""
user = [
skills_mod.Skill(name="Shared Name", instructions="USER VERSION"),
skills_mod.Skill(name="Blank First", instructions=" "),
skills_mod.Skill(name="Blank First", instructions="LATER, WITH CONTENT"),
skills_mod.Skill(name=" Padded Name ", instructions="PADDED BODY"),
skills_mod.Skill(name="MiXeD CaSe", instructions="MIXED BODY"),
]
builtin = [
skills_mod.Skill(name="Shared Name", instructions="BUILTIN VERSION"),
skills_mod.Skill(name="Builtin Only", instructions="BUILTIN BODY"),
]
_install_skills(monkeypatch, user, builtin)
harness._reload_sidebar()
expected = [(s.name, skills_mod.skill_prefix_for(s.name)) for s in user + builtin]
assert _skill_rows(harness) == expected
rows = dict(_skill_rows(harness))
# A user skill wins over a built-in of the same name: list_skills() comes
# first and skill_prefix_for() returns the FIRST match, not the last.
assert rows["Shared Name"] == "## Skill: Shared Name\nUSER VERSION"
# A blank-instructions skill does not end the scan, so its later namesake
# supplies the text. A last-write-wins dict would answer '' here.
assert rows["Blank First"] == "## Skill: Blank First\nLATER, WITH CONTENT"
assert rows["Builtin Only"] == "## Skill: Builtin Only\nBUILTIN BODY"
def test_lookup_matches_skill_prefix_for_on_odd_names(monkeypatch):
"""Slug / case / padding / unknown / empty lookups must answer like ``skill_prefix_for``."""
# Imported inside the test on purpose: this symbol is what the fix
# introduces, and a module-level import would turn the pre-fix run into a
# collection error instead of a real assertion failure in the test above.
from cowork_local.presentation.co4e.co4e_sidebar import _skill_prefix_lookup
user = [
skills_mod.Skill(name="Shared Name", instructions="USER VERSION"),
skills_mod.Skill(name="Blank First", instructions=""),
skills_mod.Skill(name="Blank First", instructions="LATER, WITH CONTENT"),
skills_mod.Skill(name="Tiếng Việt / 日本語", instructions="UNICODE BODY"),
skills_mod.Skill(name="", instructions="NAMELESS BODY"),
]
builtin = [skills_mod.Skill(name="Shared Name", instructions="BUILTIN VERSION")]
_install_skills(monkeypatch, user, builtin)
lookup = _skill_prefix_lookup(skills_mod.list_skills() + skills_mod.builtin_skills())
probes = [
"Shared Name", "shared name", "SHARED NAME", " Shared Name ",
"shared-name", # by slug
"Blank First", "blank-first",
"Tiếng Việt / 日本語", "tiếng-việt-日本語",
"", " ", "no-such-skill-anywhere", "skill",
]
for probe in probes:
assert lookup(probe) == skills_mod.skill_prefix_for(probe), probe