Fix duplicate get_by_cid method shadowing recipe lookup
Bug: Two get_by_cid methods existed in L1CacheManager. The second definition shadowed the first, breaking recipe lookup because the comprehensive method (using find_by_cid) was hidden. - Remove duplicate get_by_cid method (lines 470-494) - Add regression test to ensure only one get_by_cid exists - Add tests for template variables and recipe visibility Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
This commit is contained in:
@@ -467,32 +467,6 @@ class L1CacheManager:
|
|||||||
|
|
||||||
return None
|
return None
|
||||||
|
|
||||||
def get_by_cid(self, ipfs_cid: str) -> Optional[Path]:
|
|
||||||
"""Get cached file path by IPFS CID. Fetches from IPFS if not in local cache."""
|
|
||||||
|
|
||||||
# Check if we have this CID cached locally (indexed by CID)
|
|
||||||
cached_path = self.legacy_dir / ipfs_cid
|
|
||||||
if cached_path.exists() and cached_path.is_file():
|
|
||||||
return cached_path
|
|
||||||
|
|
||||||
# Check cache directory structure
|
|
||||||
cid_cache_dir = self.cache_dir / ipfs_cid
|
|
||||||
if cid_cache_dir.exists() and cid_cache_dir.is_dir():
|
|
||||||
# Look for output file
|
|
||||||
for f in cid_cache_dir.iterdir():
|
|
||||||
if f.is_file() and not f.name.endswith('.json'):
|
|
||||||
return f
|
|
||||||
|
|
||||||
# Fetch from IPFS
|
|
||||||
logger.info(f"Fetching from IPFS: {ipfs_cid[:16]}...")
|
|
||||||
recovery_path = self.legacy_dir / ipfs_cid
|
|
||||||
recovery_path.parent.mkdir(parents=True, exist_ok=True)
|
|
||||||
if ipfs_client.get_file(ipfs_cid, recovery_path):
|
|
||||||
logger.info(f"Fetched from IPFS: {recovery_path}")
|
|
||||||
return recovery_path
|
|
||||||
|
|
||||||
return None
|
|
||||||
|
|
||||||
def has_content(self, cid: str) -> bool:
|
def has_content(self, cid: str) -> bool:
|
||||||
"""Check if content exists in cache."""
|
"""Check if content exists in cache."""
|
||||||
return self.get_by_cid(cid) is not None
|
return self.get_by_cid(cid) is not None
|
||||||
|
|||||||
112
tests/test_recipe_visibility.py
Normal file
112
tests/test_recipe_visibility.py
Normal file
@@ -0,0 +1,112 @@
|
|||||||
|
"""
|
||||||
|
Tests for recipe visibility in web UI.
|
||||||
|
|
||||||
|
Bug found 2026-01-12: Recipes not showing in list even after upload.
|
||||||
|
"""
|
||||||
|
|
||||||
|
import pytest
|
||||||
|
from pathlib import Path
|
||||||
|
|
||||||
|
|
||||||
|
class TestRecipeListingFlow:
|
||||||
|
"""Tests for recipe listing data flow."""
|
||||||
|
|
||||||
|
def test_cache_manager_has_list_by_type(self) -> None:
|
||||||
|
"""L1CacheManager should have list_by_type method."""
|
||||||
|
# Read cache_manager.py and verify list_by_type exists
|
||||||
|
path = Path('/home/giles/art/art-celery/cache_manager.py')
|
||||||
|
content = path.read_text()
|
||||||
|
|
||||||
|
assert 'def list_by_type' in content, \
|
||||||
|
"L1CacheManager should have list_by_type method"
|
||||||
|
|
||||||
|
def test_list_by_type_returns_cid(self) -> None:
|
||||||
|
"""list_by_type should return entry.cid values."""
|
||||||
|
path = Path('/home/giles/art/art-celery/cache_manager.py')
|
||||||
|
content = path.read_text()
|
||||||
|
|
||||||
|
# Find list_by_type function and verify it appends entry.cid
|
||||||
|
assert 'hashes.append(entry.cid)' in content, \
|
||||||
|
"list_by_type should append entry.cid to results"
|
||||||
|
|
||||||
|
def test_recipe_service_uses_cache_list_by_type(self) -> None:
|
||||||
|
"""Recipe service should use cache.list_by_type('recipe')."""
|
||||||
|
path = Path('/home/giles/art/art-celery/app/services/recipe_service.py')
|
||||||
|
content = path.read_text()
|
||||||
|
|
||||||
|
assert "list_by_type('recipe')" in content, \
|
||||||
|
"Recipe service should call list_by_type with 'recipe'"
|
||||||
|
|
||||||
|
def test_recipe_upload_uses_recipe_node_type(self) -> None:
|
||||||
|
"""Recipe upload should store with node_type='recipe'."""
|
||||||
|
path = Path('/home/giles/art/art-celery/app/services/recipe_service.py')
|
||||||
|
content = path.read_text()
|
||||||
|
|
||||||
|
assert 'node_type="recipe"' in content, \
|
||||||
|
"Recipe upload should use node_type='recipe'"
|
||||||
|
|
||||||
|
def test_get_by_cid_uses_find_by_cid(self) -> None:
|
||||||
|
"""get_by_cid should use cache.find_by_cid to locate entries."""
|
||||||
|
path = Path('/home/giles/art/art-celery/cache_manager.py')
|
||||||
|
content = path.read_text()
|
||||||
|
|
||||||
|
# Verify get_by_cid uses find_by_cid
|
||||||
|
assert 'find_by_cid(cid)' in content, \
|
||||||
|
"get_by_cid should use find_by_cid to locate entries"
|
||||||
|
|
||||||
|
def test_no_duplicate_get_by_cid_methods(self) -> None:
|
||||||
|
"""
|
||||||
|
Regression test: There should only be ONE get_by_cid method.
|
||||||
|
|
||||||
|
Bug: Two get_by_cid methods existed, the second shadowed the first,
|
||||||
|
breaking recipe lookup because the comprehensive method was hidden.
|
||||||
|
"""
|
||||||
|
path = Path('/home/giles/art/art-celery/cache_manager.py')
|
||||||
|
content = path.read_text()
|
||||||
|
|
||||||
|
# Count occurrences of 'def get_by_cid'
|
||||||
|
count = content.count('def get_by_cid')
|
||||||
|
assert count == 1, \
|
||||||
|
f"Should have exactly 1 get_by_cid method, found {count}"
|
||||||
|
|
||||||
|
|
||||||
|
class TestRecipeFilterLogic:
|
||||||
|
"""Tests for recipe filtering logic."""
|
||||||
|
|
||||||
|
def test_filter_allows_none_owner(self) -> None:
|
||||||
|
"""Recipes with owner=None should be visible."""
|
||||||
|
path = Path('/home/giles/art/art-celery/app/services/recipe_service.py')
|
||||||
|
content = path.read_text()
|
||||||
|
|
||||||
|
assert 'owner is None' in content, \
|
||||||
|
"Recipe filter should allow owner=None recipes"
|
||||||
|
|
||||||
|
def test_filter_allows_matching_owner(self) -> None:
|
||||||
|
"""Recipes with matching owner should be visible."""
|
||||||
|
path = Path('/home/giles/art/art-celery/app/services/recipe_service.py')
|
||||||
|
content = path.read_text()
|
||||||
|
|
||||||
|
assert 'owner == actor_id' in content, \
|
||||||
|
"Recipe filter should allow recipes where owner matches actor_id"
|
||||||
|
|
||||||
|
|
||||||
|
class TestCacheEntryHasCid:
|
||||||
|
"""Tests for cache entry cid field."""
|
||||||
|
|
||||||
|
def test_artdag_cache_entry_has_cid(self) -> None:
|
||||||
|
"""artdag CacheEntry should have cid field."""
|
||||||
|
from artdag import CacheEntry
|
||||||
|
import dataclasses
|
||||||
|
|
||||||
|
fields = {f.name for f in dataclasses.fields(CacheEntry)}
|
||||||
|
assert 'cid' in fields, \
|
||||||
|
"CacheEntry should have cid field"
|
||||||
|
|
||||||
|
def test_artdag_cache_put_sets_cid(self) -> None:
|
||||||
|
"""artdag Cache.put should set cid on entry."""
|
||||||
|
from artdag import Cache
|
||||||
|
import inspect
|
||||||
|
|
||||||
|
source = inspect.getsource(Cache.put)
|
||||||
|
assert 'cid=' in source, \
|
||||||
|
"Cache.put should set cid on entry"
|
||||||
@@ -1,5 +1,5 @@
|
|||||||
"""
|
"""
|
||||||
Tests for run artifacts data structure.
|
Tests for run artifacts data structure and template variables.
|
||||||
|
|
||||||
Bug found 2026-01-12: runs/detail.html template expects artifact.cid
|
Bug found 2026-01-12: runs/detail.html template expects artifact.cid
|
||||||
but the route provides artifact.hash, causing UndefinedError.
|
but the route provides artifact.hash, causing UndefinedError.
|
||||||
@@ -9,6 +9,33 @@ import pytest
|
|||||||
from pathlib import Path
|
from pathlib import Path
|
||||||
|
|
||||||
|
|
||||||
|
class TestCacheNotFoundTemplate:
|
||||||
|
"""Tests for cache/not_found.html template."""
|
||||||
|
|
||||||
|
def test_template_uses_cid_not_content_hash(self) -> None:
|
||||||
|
"""
|
||||||
|
Regression test: not_found.html must use 'cid' variable.
|
||||||
|
|
||||||
|
Bug: Template used 'content_hash' but route passes 'cid'.
|
||||||
|
Fix: Changed template to use 'cid'.
|
||||||
|
"""
|
||||||
|
template_path = Path('/home/giles/art/art-celery/app/templates/cache/not_found.html')
|
||||||
|
content = template_path.read_text()
|
||||||
|
|
||||||
|
assert 'cid' in content, "Template should use 'cid' variable"
|
||||||
|
assert 'content_hash' not in content, \
|
||||||
|
"Template should not use 'content_hash' (route passes 'cid')"
|
||||||
|
|
||||||
|
def test_route_passes_cid_to_template(self) -> None:
|
||||||
|
"""Verify route passes 'cid' variable to not_found template."""
|
||||||
|
router_path = Path('/home/giles/art/art-celery/app/routers/cache.py')
|
||||||
|
content = router_path.read_text()
|
||||||
|
|
||||||
|
# Find the render call for not_found.html
|
||||||
|
assert 'cid=cid' in content, \
|
||||||
|
"Route should pass cid=cid to not_found.html template"
|
||||||
|
|
||||||
|
|
||||||
class TestArtifactDataStructure:
|
class TestArtifactDataStructure:
|
||||||
"""Tests for artifact dict keys matching template expectations."""
|
"""Tests for artifact dict keys matching template expectations."""
|
||||||
|
|
||||||
|
|||||||
Reference in New Issue
Block a user