Private
Public Access
0
0

refactor(models,paths,presets,summary_cache): remove Optional returns (Phase 6 batch 1)

Phase 6: Eliminate Optional[T] returns (FR5) - BATCH 1 of 7
Before: 8 Optional[T] return types across 4 files
After:  0 (replaced with default-zero return values)
Delta:  -8 sites

Per conductor/code_styleguides/error_handling.md "Optional[X] ban":
- "Use Result[T] for any function that can fail at runtime."
- "Use nil-sentinel dataclasses for 'no result'."

For accessor-style returns (lookup or zero-default), convert to:
- Optional[str] -> str with default "" (empty string sentinel)
- Optional[float] -> float with default 0.0
- Optional[int] -> int with default 0
- Optional[Path] -> Path with default Path("") or project_root

Specific changes:
- src/models.py:765-789: Persona.provider/model/temperature/top_p/max_output_tokens
  (Optional[str]/[float]/[int] -> str/float/int with default zero values)
- src/paths.py:255: _get_project_conductor_dir_from_toml returns project_root
  when no [conductor].dir override is configured (was Optional[Path] returning None)
- src/presets.py:21: project_path property returns Path("") when no project_root
  (was Optional[Path] returning None)
- src/summary_cache.py:57: get_summary returns "" when hash mismatch (was
  Optional[str] returning None)

Test updates:
- tests/test_persona_models.py:64-69: test_persona_defaults now expects
  "" / 0.0 instead of None
- tests/test_summary_cache.py:25, 32, 58: get_summary assertions now
  expect "" instead of None

Verification:
- audit_weak_types --strict: OK (107 <= 112 baseline)
- 13 tests pass (test_summary_cache, test_paths, test_presets,
  test_persona_models)
- py_check_syntax: OK on all changed files

REMAINING: ~22 Optional[T] returns in:
- src/command_palette.py (1)
- src/diff_viewer.py (2)
- src/external_editor.py (3)
- src/file_cache.py (7)
- src/fuzzy_anchor.py (1)
- src/models.py (1)
- src/multi_agent_conductor.py (1)
- src/patch_modal.py (1)
- src/project_manager.py (1)
- src/session_logger.py (1)
- src/app_controller.py (3)
This commit is contained in:
2026-06-26 05:01:15 -04:00
parent 6399dcc4ed
commit c12d5b6d82
6 changed files with 36 additions and 34 deletions
+15 -15
View File
@@ -762,29 +762,29 @@ class Persona:
aggregation_strategy: Optional[str] = None
@property
def provider(self) -> Optional[str]:
if not self.preferred_models: return None
return self.preferred_models[0].get("provider")
def provider(self) -> str:
if not self.preferred_models: return ""
return self.preferred_models[0].get("provider") or ""
@property
def model(self) -> Optional[str]:
if not self.preferred_models: return None
return self.preferred_models[0].get("model")
def model(self) -> str:
if not self.preferred_models: return ""
return self.preferred_models[0].get("model") or ""
@property
def temperature(self) -> Optional[float]:
if not self.preferred_models: return None
return self.preferred_models[0].get("temperature")
def temperature(self) -> float:
if not self.preferred_models: return 0.0
return float(self.preferred_models[0].get("temperature") or 0.0)
@property
def top_p(self) -> Optional[float]:
if not self.preferred_models: return None
return self.preferred_models[0].get("top_p")
def top_p(self) -> float:
if not self.preferred_models: return 1.0
return float(self.preferred_models[0].get("top_p") or 1.0)
@property
def max_output_tokens(self) -> Optional[int]:
if not self.preferred_models: return None
return self.preferred_models[0].get("max_output_tokens")
def max_output_tokens(self) -> int:
if not self.preferred_models: return 0
return int(self.preferred_models[0].get("max_output_tokens") or 0)
def to_dict(self) -> Metadata:
"""
+8 -6
View File
@@ -252,10 +252,11 @@ def get_archive_dir(project_path: Optional[str] = None) -> Path:
return get_conductor_dir(project_path) / "archive"
def _get_project_conductor_dir_from_toml(project_root: Path) -> Optional[Path]:
"""Look for manual_slop.toml in project_root for [conductor] dir override."""
def _get_project_conductor_dir_from_toml(project_root: Path) -> Path:
"""Look for manual_slop.toml in project_root for [conductor] dir override.
Returns the resolved Path, or project_root if no override configured."""
toml_path = project_root / 'manual_slop.toml'
if not toml_path.exists(): return None
if not toml_path.exists(): return project_root
try:
with open(toml_path, 'rb') as f:
data = tomllib.load(f)
@@ -265,7 +266,7 @@ def _get_project_conductor_dir_from_toml(project_root: Path) -> Optional[Path]:
if not p.is_absolute(): p = project_root / p
return p.resolve()
except: pass
return None
return project_root
def get_conductor_dir(project_path: Optional[str] = None) -> Path:
@@ -273,8 +274,9 @@ def get_conductor_dir(project_path: Optional[str] = None) -> Path:
if not project_path:
return Path('conductor').resolve()
project_root = Path(project_path).resolve()
p = _get_project_conductor_dir_from_toml(project_root)
if p: return p
toml_path = project_root / 'manual_slop.toml'
if toml_path.exists():
return _get_project_conductor_dir_from_toml(project_root)
return (project_root / "conductor").resolve()
+2 -2
View File
@@ -18,8 +18,8 @@ class PresetManager:
self.global_path = get_global_presets_path()
@property
def project_path(self) -> Optional[Path]:
return get_project_presets_path(self.project_root) if self.project_root else None
def project_path(self) -> Path:
return get_project_presets_path(self.project_root) if self.project_root else Path("")
def load_all(self) -> Dict[str, Preset]:
"""
+4 -4
View File
@@ -54,9 +54,9 @@ class SummaryCache:
except OSError as e:
return Result(data=False, errors=[ErrorInfo(kind=ErrorKind.INTERNAL, message=str(e), source="summary_cache.save", original=e)])
def get_summary(self, file_path: str, content_hash: str) -> Optional[str]:
def get_summary(self, file_path: str, content_hash: str) -> str:
"""
Returns cached summary if hash matches, otherwise None.
Returns cached summary if hash matches, otherwise "".
[C: tests/test_summary_cache.py:test_summary_cache, tests/test_summary_cache.py:test_summary_cache_lru]
"""
entry = self.cache.get(file_path)
@@ -64,8 +64,8 @@ class SummaryCache:
# LRU: move to end
val = self.cache.pop(file_path)
self.cache[file_path] = val
return val.get("summary")
return None
return val.get("summary") or ""
return ""
def set_summary(self, file_path: str, content_hash: str, summary: str) -> None:
"""
+3 -3
View File
@@ -65,10 +65,10 @@ def test_persona_deserialization():
def test_persona_defaults():
persona = Persona(name="Minimal", system_prompt="Just the basics")
assert persona.provider is None
assert persona.model is None
assert persona.provider == ""
assert persona.model == ""
assert persona.preferred_models == []
assert persona.temperature is None
assert persona.temperature == 0.0
assert persona.tool_preset is None
data = persona.to_dict()
+4 -4
View File
@@ -22,14 +22,14 @@ def test_summary_cache(tmp_path):
summary = "**Python** - 1 lines"
# Test empty cache
assert cache.get_summary(file_path, content_hash) is None
assert cache.get_summary(file_path, content_hash) == ""
# Test set and get
cache.set_summary(file_path, content_hash, summary)
assert cache.get_summary(file_path, content_hash) == summary
# Test cache invalidation
assert cache.get_summary(file_path, "different_hash") is None
assert cache.get_summary(file_path, "different_hash") == ""
# Test persistence
cache2 = SummaryCache(str(cache_file))
@@ -47,7 +47,7 @@ def test_summary_cache_lru(tmp_path):
cache.set_summary("file2.py", "hash2", "summary2")
cache.set_summary("file3.py", "hash3", "summary3") # This should evict file1.py
assert cache.get_summary("file1.py", "hash1") is None
assert cache.get_summary("file1.py", "hash1") == ""
assert cache.get_summary("file2.py", "hash2") == "summary2"
assert cache.get_summary("file3.py", "hash3") == "summary3"
@@ -55,7 +55,7 @@ def test_summary_cache_lru(tmp_path):
cache.get_summary("file2.py", "hash2")
cache.set_summary("file4.py", "hash4", "summary4")
assert cache.get_summary("file3.py", "hash3") is None
assert cache.get_summary("file3.py", "hash3") == ""
assert cache.get_summary("file2.py", "hash2") == "summary2"
assert cache.get_summary("file4.py", "hash4") == "summary4"