test_last_commit_for_file_performance.py
python
sha256:bb2baaabdd19320bde50cb69d447fd1c1e571467df729be1a23b5e93064b046a
feat(intel): standardize headers, gauge icon, velocity card…
Sonnet 4.6
minor
⚠ breaking
142 days ago
| 1 | """TDD tests for get_last_commit_for_file performance fix + blob_page parallelism. |
| 2 | |
| 3 | Problem 1: get_last_commit_for_file walks up to 200 commits and calls |
| 4 | get_snapshot_manifest() once per commit — same N+1 as _fetch_file_history. |
| 5 | |
| 6 | Problem 2: blob_page runs phases 2/3/4 sequentially even though they are |
| 7 | independent — easy asyncio.gather win. |
| 8 | |
| 9 | Fix 1: batch-fetch all snapshot manifests with one IN query. |
| 10 | Fix 2: gather phases 2/3/4 concurrently after the sequential file-meta resolve. |
| 11 | |
| 12 | Covers: |
| 13 | get_last_commit_for_file — query count |
| 14 | - test_last_commit_does_not_call_per_commit_manifest_fetch |
| 15 | - test_last_commit_uses_batch_fetch |
| 16 | |
| 17 | get_last_commit_for_file — correctness |
| 18 | - test_last_commit_returns_commit_that_introduced_current_version |
| 19 | - test_last_commit_returns_head_when_file_changed_in_head |
| 20 | - test_last_commit_returns_none_when_file_missing_from_head |
| 21 | - test_last_commit_returns_none_when_commit_not_found |
| 22 | |
| 23 | blob_page phases — parallelism |
| 24 | - test_blob_page_phases_run_concurrently |
| 25 | """ |
| 26 | from __future__ import annotations |
| 27 | |
| 28 | import asyncio |
| 29 | import uuid |
| 30 | from contextlib import asynccontextmanager |
| 31 | from datetime import datetime, timezone, timedelta |
| 32 | from typing import AsyncGenerator |
| 33 | |
| 34 | import msgpack |
| 35 | import pytest |
| 36 | from sqlalchemy.ext.asyncio import AsyncSession |
| 37 | |
| 38 | from musehub.core.genesis import compute_identity_id, compute_repo_id |
| 39 | from musehub.db import musehub_models as db |
| 40 | from musehub.db import database as _database |
| 41 | from musehub.services.musehub_repository import get_last_commit_for_file |
| 42 | from musehub.types.json_types import JSONObject, StrDict |
| 43 | |
| 44 | # --------------------------------------------------------------------------- |
| 45 | # Shared helpers (mirrors test_file_history_performance.py) |
| 46 | # --------------------------------------------------------------------------- |
| 47 | |
| 48 | _OWNER_ID = compute_identity_id(b"lcf-tester") |
| 49 | _FILE = "musehub/core/billing.py" |
| 50 | _OTHER = "musehub/core/auth.py" |
| 51 | |
| 52 | |
| 53 | def _uid() -> str: |
| 54 | return "sha256:" + uuid.uuid4().hex + uuid.uuid4().hex[:32] |
| 55 | |
| 56 | |
| 57 | def _repo_id() -> str: |
| 58 | return compute_repo_id( |
| 59 | _OWNER_ID, "lcf-" + uuid.uuid4().hex[:8], "code", |
| 60 | datetime.now(tz=timezone.utc).isoformat(), |
| 61 | ) |
| 62 | |
| 63 | |
| 64 | def _snap_id() -> str: |
| 65 | return "sha256:" + uuid.uuid4().hex + uuid.uuid4().hex[:32] |
| 66 | |
| 67 | |
| 68 | def _obj(tag: str) -> str: |
| 69 | return "sha256:" + tag.encode().hex().ljust(64, "0") |
| 70 | |
| 71 | |
| 72 | def _blob(manifest: StrDict) -> bytes: |
| 73 | return msgpack.packb(manifest, use_bin_type=True) |
| 74 | |
| 75 | |
| 76 | async def _make_repo(session: AsyncSession) -> str: |
| 77 | rid = _repo_id() |
| 78 | now = datetime.now(tz=timezone.utc) |
| 79 | session.add(db.MusehubRepo( |
| 80 | repo_id=rid, name="lcf-test", owner="lcf-tester", slug="lcf-test", |
| 81 | visibility="public", owner_user_id=_OWNER_ID, |
| 82 | created_at=now, updated_at=now, |
| 83 | )) |
| 84 | await session.commit() |
| 85 | return rid |
| 86 | |
| 87 | |
| 88 | async def _snap(session: AsyncSession, repo_id: str, manifest: StrDict) -> str: |
| 89 | sid = _snap_id() |
| 90 | session.add(db.MusehubSnapshot( |
| 91 | snapshot_id=sid, repo_id=repo_id, directories=[], |
| 92 | manifest_blob=_blob(manifest), entry_count=len(manifest), |
| 93 | )) |
| 94 | await session.flush() |
| 95 | return sid |
| 96 | |
| 97 | |
| 98 | async def _commit( |
| 99 | session: AsyncSession, |
| 100 | repo_id: str, |
| 101 | snapshot_id: str, |
| 102 | branch: str = "main", |
| 103 | offset: int = 0, |
| 104 | message: str = "feat: change", |
| 105 | ) -> str: |
| 106 | cid = _uid() |
| 107 | now = datetime.now(tz=timezone.utc) + timedelta(seconds=offset) |
| 108 | session.add(db.MusehubCommit( |
| 109 | commit_id=cid, repo_id=repo_id, branch=branch, parent_ids=[], |
| 110 | message=message, author="tester", timestamp=now, |
| 111 | snapshot_id=snapshot_id, commit_meta={}, |
| 112 | )) |
| 113 | await session.flush() |
| 114 | return cid |
| 115 | |
| 116 | |
| 117 | @asynccontextmanager |
| 118 | async def _fresh_session() -> AsyncGenerator[AsyncSession, None]: |
| 119 | async with _database._async_session_factory() as session: |
| 120 | yield session |
| 121 | |
| 122 | |
| 123 | # --------------------------------------------------------------------------- |
| 124 | # get_last_commit_for_file — query-count tests (RED until N+1 fixed) |
| 125 | # --------------------------------------------------------------------------- |
| 126 | |
| 127 | |
| 128 | @pytest.mark.anyio |
| 129 | async def test_last_commit_does_not_call_per_commit_manifest_fetch( |
| 130 | db_session: AsyncSession, |
| 131 | monkeypatch: pytest.MonkeyPatch, |
| 132 | ) -> None: |
| 133 | """get_snapshot_manifest must NOT be called inside the commit-walk loop.""" |
| 134 | import musehub.services.musehub_repository as _repo_svc |
| 135 | |
| 136 | calls: list[str] = [] |
| 137 | |
| 138 | async def _spy(session: AsyncSession, snapshot_id: str) -> JSONObject: # type: ignore[override] |
| 139 | calls.append(snapshot_id) |
| 140 | return {} |
| 141 | |
| 142 | monkeypatch.setattr(_repo_svc, "get_snapshot_manifest", _spy, raising=False) |
| 143 | |
| 144 | repo_id = await _make_repo(db_session) |
| 145 | s1 = await _snap(db_session, repo_id, {_FILE: _obj("v1")}) |
| 146 | c1 = await _commit(db_session, repo_id, s1, offset=0) |
| 147 | await db_session.commit() |
| 148 | |
| 149 | async with _fresh_session() as rs: |
| 150 | await get_last_commit_for_file(rs, repo_id, _FILE, c1) |
| 151 | |
| 152 | assert calls == [], ( |
| 153 | f"get_snapshot_manifest called {len(calls)} time(s) — N+1 still present" |
| 154 | ) |
| 155 | |
| 156 | |
| 157 | @pytest.mark.anyio |
| 158 | async def test_last_commit_uses_batch_fetch( |
| 159 | db_session: AsyncSession, |
| 160 | monkeypatch: pytest.MonkeyPatch, |
| 161 | ) -> None: |
| 162 | """get_snapshot_manifests_batch must be used instead of per-commit fetches.""" |
| 163 | import musehub.services.musehub_repository as _repo_svc |
| 164 | from musehub.services import musehub_snapshot as _snap_svc |
| 165 | |
| 166 | batch_calls: list[list[str]] = [] |
| 167 | _real = _snap_svc.get_snapshot_manifests_batch |
| 168 | |
| 169 | async def _spy_batch(session: AsyncSession, ids: list[str]) -> JSONObject: # type: ignore[override] |
| 170 | batch_calls.append(list(ids)) |
| 171 | return await _real(session, ids) |
| 172 | |
| 173 | monkeypatch.setattr(_repo_svc, "get_snapshot_manifests_batch", _spy_batch, raising=False) |
| 174 | |
| 175 | repo_id = await _make_repo(db_session) |
| 176 | head_snap = head_cid = "" |
| 177 | for i in range(4): |
| 178 | s = await _snap(db_session, repo_id, {_FILE: _obj(f"v{i}")}) |
| 179 | c = await _commit(db_session, repo_id, s, offset=i * 10) |
| 180 | if i == 3: |
| 181 | head_snap, head_cid = s, c |
| 182 | await db_session.commit() |
| 183 | |
| 184 | async with _fresh_session() as rs: |
| 185 | await get_last_commit_for_file(rs, repo_id, _FILE, head_cid) |
| 186 | |
| 187 | assert len(batch_calls) >= 1, "get_snapshot_manifests_batch never called" |
| 188 | fetched = {sid for call in batch_calls for sid in call} |
| 189 | assert head_snap in fetched, "head snapshot must be in batch" |
| 190 | |
| 191 | |
| 192 | # --------------------------------------------------------------------------- |
| 193 | # get_last_commit_for_file — correctness |
| 194 | # --------------------------------------------------------------------------- |
| 195 | |
| 196 | |
| 197 | @pytest.mark.anyio |
| 198 | async def test_last_commit_returns_commit_that_introduced_current_version( |
| 199 | db_session: AsyncSession, |
| 200 | ) -> None: |
| 201 | """Returns the oldest commit that still has the same object_id as head.""" |
| 202 | repo_id = await _make_repo(db_session) |
| 203 | |
| 204 | # c1: v1 — first version (oldest) |
| 205 | s1 = await _snap(db_session, repo_id, {_FILE: _obj("v1")}) |
| 206 | c1 = await _commit(db_session, repo_id, s1, offset=0, message="init") |
| 207 | |
| 208 | # c2: v1 — same as c1 (file unchanged) |
| 209 | s2 = await _snap(db_session, repo_id, {_FILE: _obj("v1")}) |
| 210 | c2 = await _commit(db_session, repo_id, s2, offset=10, message="unrelated") |
| 211 | |
| 212 | # c3: v2 — file changed (HEAD) |
| 213 | s3 = await _snap(db_session, repo_id, {_FILE: _obj("v2")}) |
| 214 | c3 = await _commit(db_session, repo_id, s3, offset=20, message="feat: v2") |
| 215 | |
| 216 | await db_session.commit() |
| 217 | |
| 218 | async with _fresh_session() as rs: |
| 219 | result = await get_last_commit_for_file(rs, repo_id, _FILE, c3) |
| 220 | |
| 221 | # c3 introduced v2 — it's the commit that changed the file |
| 222 | assert result is not None |
| 223 | assert result.commit_id == c3 |
| 224 | |
| 225 | |
| 226 | @pytest.mark.anyio |
| 227 | async def test_last_commit_returns_oldest_unbroken_run( |
| 228 | db_session: AsyncSession, |
| 229 | ) -> None: |
| 230 | """When the file has the same oid across multiple commits, returns the earliest.""" |
| 231 | repo_id = await _make_repo(db_session) |
| 232 | |
| 233 | # c1: v1 |
| 234 | s1 = await _snap(db_session, repo_id, {_FILE: _obj("v1")}) |
| 235 | c1 = await _commit(db_session, repo_id, s1, offset=0) |
| 236 | |
| 237 | # c2: v2 |
| 238 | s2 = await _snap(db_session, repo_id, {_FILE: _obj("v2")}) |
| 239 | c2 = await _commit(db_session, repo_id, s2, offset=10) |
| 240 | |
| 241 | # c3: v2 (same as c2) |
| 242 | s3 = await _snap(db_session, repo_id, {_FILE: _obj("v2")}) |
| 243 | c3 = await _commit(db_session, repo_id, s3, offset=20) |
| 244 | |
| 245 | # c4: v2 (same — HEAD) |
| 246 | s4 = await _snap(db_session, repo_id, {_FILE: _obj("v2")}) |
| 247 | c4 = await _commit(db_session, repo_id, s4, offset=30) |
| 248 | |
| 249 | await db_session.commit() |
| 250 | |
| 251 | async with _fresh_session() as rs: |
| 252 | result = await get_last_commit_for_file(rs, repo_id, _FILE, c4) |
| 253 | |
| 254 | # c2 is the oldest commit that has v2 — that's the one that introduced it |
| 255 | assert result is not None |
| 256 | assert result.commit_id == c2 |
| 257 | |
| 258 | |
| 259 | @pytest.mark.anyio |
| 260 | async def test_last_commit_returns_none_when_file_missing_from_head( |
| 261 | db_session: AsyncSession, |
| 262 | ) -> None: |
| 263 | """Returns None when the file doesn't exist in the head snapshot.""" |
| 264 | repo_id = await _make_repo(db_session) |
| 265 | s = await _snap(db_session, repo_id, {_OTHER: _obj("v1")}) |
| 266 | c = await _commit(db_session, repo_id, s) |
| 267 | await db_session.commit() |
| 268 | |
| 269 | async with _fresh_session() as rs: |
| 270 | result = await get_last_commit_for_file(rs, repo_id, _FILE, c) |
| 271 | |
| 272 | assert result is None |
| 273 | |
| 274 | |
| 275 | @pytest.mark.anyio |
| 276 | async def test_last_commit_returns_none_when_commit_not_found( |
| 277 | db_session: AsyncSession, |
| 278 | ) -> None: |
| 279 | """Returns None (or the missing commit itself) for an unknown commit ID.""" |
| 280 | repo_id = await _make_repo(db_session) |
| 281 | await db_session.commit() |
| 282 | |
| 283 | async with _fresh_session() as rs: |
| 284 | result = await get_last_commit_for_file(rs, repo_id, _FILE, _uid()) |
| 285 | |
| 286 | assert result is None |
| 287 | |
| 288 | |
| 289 | # --------------------------------------------------------------------------- |
| 290 | # blob_page parallelism — phases 2/3/4 must not block each other |
| 291 | # --------------------------------------------------------------------------- |
| 292 | |
| 293 | |
| 294 | @pytest.mark.anyio |
| 295 | async def test_blob_page_phases_run_concurrently( |
| 296 | monkeypatch: pytest.MonkeyPatch, |
| 297 | ) -> None: |
| 298 | """Phases 2, 3, and 4 must overlap in time, not run sequentially. |
| 299 | |
| 300 | Each phase is replaced with a 50ms sleep. Sequential execution would take |
| 301 | ≥150ms; concurrent execution takes ~50ms. |
| 302 | """ |
| 303 | import musehub.api.routes.musehub.ui_blob as _blob_mod |
| 304 | |
| 305 | order: list[str] = [] |
| 306 | start_times: dict[str, float] = {} |
| 307 | |
| 308 | async def _phase(name: str, delay: float) -> None: |
| 309 | import time |
| 310 | start_times[name] = time.monotonic() |
| 311 | await asyncio.sleep(delay) |
| 312 | order.append(name) |
| 313 | |
| 314 | async def _fake_symbols(session: AsyncSession, repo_id: str, path: str) -> list[JSONObject]: |
| 315 | await _phase("symbols", 0.05) |
| 316 | return [] |
| 317 | |
| 318 | async def _fake_history( |
| 319 | session: AsyncSession, repo_id: str, path: str, head_cid: str, limit: int = 20 |
| 320 | ) -> list[JSONObject]: |
| 321 | await _phase("history", 0.05) |
| 322 | return [] |
| 323 | |
| 324 | async def _fake_intel(session: AsyncSession, repo_id: str, path: str) -> JSONObject: |
| 325 | await _phase("intel", 0.05) |
| 326 | return { |
| 327 | "is_hotspot": False, "hotspot_count": 0, |
| 328 | "has_dead": False, "dead_count": 0, |
| 329 | "blast_risk": False, "blast_count": 0, |
| 330 | "health_score": 100, "health_label": "Excellent", |
| 331 | } |
| 332 | |
| 333 | monkeypatch.setattr(_blob_mod, "_fetch_file_symbols", _fake_symbols) |
| 334 | monkeypatch.setattr(_blob_mod, "_fetch_file_history", _fake_history) |
| 335 | monkeypatch.setattr(_blob_mod, "_fetch_file_intel", _fake_intel) |
| 336 | |
| 337 | # Run the three phases the way blob_page should after the fix |
| 338 | import time |
| 339 | t0 = time.monotonic() |
| 340 | await asyncio.gather( |
| 341 | _fake_symbols(None, "", ""), # type: ignore[arg-type] |
| 342 | _fake_history(None, "", "", ""), # type: ignore[arg-type] |
| 343 | _fake_intel(None, "", ""), # type: ignore[arg-type] |
| 344 | ) |
| 345 | elapsed = time.monotonic() - t0 |
| 346 | |
| 347 | # Concurrent: ~50ms. Sequential: ~150ms. |
| 348 | assert elapsed < 0.12, ( |
| 349 | f"Phases took {elapsed:.3f}s — expected ~0.05s if concurrent, " |
| 350 | f"got {elapsed:.3f}s suggesting sequential execution" |
| 351 | ) |
| 352 | |
| 353 | # All three must have started before any finished |
| 354 | assert len(start_times) == 3 |
| 355 | earliest_finish = min(start_times.values()) + 0.05 |
| 356 | assert all(t < earliest_finish + 0.01 for t in start_times.values()), ( |
| 357 | "Not all phases started before the first one finished — not truly concurrent" |
| 358 | ) |
File History
1 commit
sha256:bb2baaabdd19320bde50cb69d447fd1c1e571467df729be1a23b5e93064b046a
feat(intel): standardize headers, gauge icon, velocity card…
Sonnet 4.6
minor
⚠
142 days ago