diff --git a/apps/import/call.py b/apps/import/call.py index 51f4ae6a1..81b0b3fa4 100644 --- a/apps/import/call.py +++ b/apps/import/call.py @@ -464,18 +464,21 @@ def resolve_entity( typer.echo(f"Skipped staged entity {source_id}.") -@app.command("resolve-facet") -def resolve_facet( +@app.command("resolve-staged-facet") +def apply_staged_facet( staged_file: str = typer.Argument( help="Staged file path relative to facets/staged/." ), - action: str = typer.Argument(help="Action: apply or skip."), + apply: bool = typer.Option( + False, "--apply", help="Apply the staged item into the journal." + ), + skip: bool = typer.Option(False, "--skip", help="Discard the staged item."), source: str = typer.Option(..., "--source", help="Import source name."), ) -> None: - _, _, state_dir = _resolve_source(source) + if apply == skip: + _fail("Exactly one of --apply or --skip is required.") - if action not in {"apply", "skip"}: - _fail("Action must be 'apply' or 'skip'.") + _, _, state_dir = _resolve_source(source) staged_dir = state_dir / "facets" / "staged" staged_path = staged_dir / staged_file @@ -500,7 +503,7 @@ def resolve_facet( else: item_id = f"{facet_name}/{payload.get('source_path', staged_file)}" - if action == "skip": + if skip: staged_path.unlink() _log_resolution( log_path, diff --git a/apps/import/talent/review.md b/apps/import/talent/review.md index 77b7e4b9e..92a01d310 100644 --- a/apps/import/talent/review.md +++ b/apps/import/talent/review.md @@ -21,8 +21,8 @@ Run the full import review pipeline in the correct dependency order: entities fi - `sol call import resolve-entity SOURCE_ID merge --source SOURCE --target TARGET_ID` - `sol call import resolve-entity SOURCE_ID create --source SOURCE` - `sol call import resolve-entity SOURCE_ID skip --source SOURCE` -- `sol call import resolve-facet STAGED_FILE apply --source SOURCE` -- `sol call import resolve-facet STAGED_FILE skip --source SOURCE` +- `sol call import resolve-staged-facet STAGED_FILE --apply --source SOURCE` +- `sol call import resolve-staged-facet STAGED_FILE --skip --source SOURCE` - `sol call import resolve-config FIELD apply --source SOURCE` - `sol call import resolve-config FIELD keep --source SOURCE` - `sol call import resolve-config-all --source SOURCE --category transferable` diff --git a/apps/import/talent/review_facets.md b/apps/import/talent/review_facets.md index 845690e55..ecd65e53b 100644 --- a/apps/import/talent/review_facets.md +++ b/apps/import/talent/review_facets.md @@ -15,8 +15,8 @@ Review staged facet items and resolve them. Entity review must complete first be ## Tooling - `sol call import list-staged --source SOURCE --area facets` - list staged facet items as JSONL -- `sol call import resolve-facet STAGED_FILE apply --source SOURCE` - apply a staged facet item -- `sol call import resolve-facet STAGED_FILE skip --source SOURCE` - discard a staged facet item +- `sol call import resolve-staged-facet STAGED_FILE --apply --source SOURCE` - apply a staged facet item +- `sol call import resolve-staged-facet STAGED_FILE --skip --source SOURCE` - discard a staged facet item - `sol call import list-staged --source SOURCE --area entities` - check whether entity review is complete ## Process @@ -57,13 +57,13 @@ Parse the JSONL output and review each staged item. - Apply: ```bash -sol call import resolve-facet STAGED_FILE apply --source SOURCE +sol call import resolve-staged-facet STAGED_FILE --apply --source SOURCE ``` - Skip: ```bash -sol call import resolve-facet STAGED_FILE skip --source SOURCE +sol call import resolve-staged-facet STAGED_FILE --skip --source SOURCE ``` ### Step 6: Verify and Report diff --git a/apps/settings/call.py b/apps/settings/call.py index e9ec0eff6..b80c3d9af 100644 --- a/apps/settings/call.py +++ b/apps/settings/call.py @@ -229,8 +229,12 @@ def keys_clear( @keys_app.command("validate") -def keys_validate() -> None: - """Re-validate all configured API keys.""" +def keys_validate( + cache_result: bool = typer.Option( + False, "--cache-result", help="Persist results to providers.key_validation." + ), +) -> None: + """Validate all configured API keys without persisting by default.""" from think.providers import PROVIDER_METADATA, validate_key from think.providers.google import validate_vertex_credentials @@ -258,9 +262,10 @@ def keys_validate() -> None: result["timestamp"] = datetime.now(timezone.utc).isoformat() key_validation["google"] = result - config.setdefault("providers", {}) - config["providers"]["key_validation"] = key_validation - _write_config(config) + if cache_result: + config.setdefault("providers", {}) + config["providers"]["key_validation"] = key_validation + _write_config(config) typer.echo(json.dumps({"key_validation": key_validation}, indent=2)) diff --git a/apps/settings/tests/__init__.py b/apps/settings/tests/__init__.py deleted file mode 100644 index 8b1378917..000000000 --- a/apps/settings/tests/__init__.py +++ /dev/null @@ -1 +0,0 @@ - diff --git a/apps/settings/tests/conftest.py b/apps/settings/tests/conftest.py index 0aad8e659..1835c278f 100644 --- a/apps/settings/tests/conftest.py +++ b/apps/settings/tests/conftest.py @@ -10,6 +10,12 @@ import json import pytest +@pytest.fixture(autouse=True) +def _skip_supervisor_check(monkeypatch): + """Allow app CLI tests to run without a live solstone supervisor.""" + monkeypatch.setenv("SOL_SKIP_SUPERVISOR_CHECK", "1") + + @pytest.fixture def settings_env(tmp_path, monkeypatch): """Create a temporary journal with settings config.""" diff --git a/apps/settings/tests/test_call.py b/apps/settings/tests/test_call.py index 63e0f1c8a..87dc8186f 100644 --- a/apps/settings/tests/test_call.py +++ b/apps/settings/tests/test_call.py @@ -7,6 +7,7 @@ from __future__ import annotations import json import os +from datetime import datetime, timezone from unittest.mock import patch from typer.testing import CliRunner @@ -87,6 +88,96 @@ class TestKeysClear: assert saved["providers"]["auth"]["google"] == "platform" +class TestKeysValidate: + class _FixedDateTime: + @classmethod + def now(cls, tz=None): + return datetime(2026, 4, 17, 12, 0, tzinfo=tz or timezone.utc) + + def test_keys_validate_default_does_not_write_config(self, settings_env): + tmp_path, _config = settings_env() + config_path = tmp_path / "config" / "journal.json" + before = config_path.read_text(encoding="utf-8") + + with ( + patch("apps.settings.call.datetime", self._FixedDateTime), + patch( + "think.providers.validate_key", + side_effect=[ + {"valid": True, "provider": "google"}, + {"valid": True, "provider": "openai"}, + ], + ), + ): + result = runner.invoke(call_app, ["settings", "keys", "validate"]) + + assert result.exit_code == 0 + assert config_path.read_text(encoding="utf-8") == before + payload = json.loads(result.output) + assert payload == { + "key_validation": { + "google": { + "valid": True, + "provider": "google", + "timestamp": "2026-04-17T12:00:00+00:00", + }, + "openai": { + "valid": True, + "provider": "openai", + "timestamp": "2026-04-17T12:00:00+00:00", + }, + } + } + + def test_keys_validate_cache_result_persists(self, settings_env): + tmp_path, _config = settings_env() + config_path = tmp_path / "config" / "journal.json" + + with ( + patch("apps.settings.call.datetime", self._FixedDateTime), + patch( + "think.providers.validate_key", + side_effect=[ + {"valid": True, "provider": "google"}, + {"valid": True, "provider": "openai"}, + ], + ), + ): + result = runner.invoke( + call_app, + ["settings", "keys", "validate", "--cache-result"], + ) + + assert result.exit_code == 0 + assert json.loads(result.output) == { + "key_validation": { + "google": { + "valid": True, + "provider": "google", + "timestamp": "2026-04-17T12:00:00+00:00", + }, + "openai": { + "valid": True, + "provider": "openai", + "timestamp": "2026-04-17T12:00:00+00:00", + }, + } + } + saved = json.loads(config_path.read_text(encoding="utf-8")) + assert saved["providers"]["key_validation"] == { + "google": { + "valid": True, + "provider": "google", + "timestamp": "2026-04-17T12:00:00+00:00", + }, + "openai": { + "valid": True, + "provider": "openai", + "timestamp": "2026-04-17T12:00:00+00:00", + }, + } + + class TestProvidersShow: def test_providers_show(self, settings_env): settings_env() diff --git a/apps/speakers/call.py b/apps/speakers/call.py index 6dcf6ff0c..d4d7fd2c9 100644 --- a/apps/speakers/call.py +++ b/apps/speakers/call.py @@ -3,20 +3,22 @@ """CLI interface for speaker voiceprint management. -Provides: +Speaker writer commands preview by default; pass ``--commit`` to persist. +For ``attribute-segment``, ``--save`` / ``--accumulate`` only take effect +when ``--commit`` is also passed. + +Commands: sol call speakers status [section] - sol call speakers bootstrap [--dry-run] [--json] - sol call speakers resolve-names [--dry-run] [--json] - sol call speakers attribute-segment [--json] - sol call speakers backfill [--dry-run] [--json] + sol call speakers bootstrap [--commit] [--json] + sol call speakers resolve-names [--commit] [--json] + sol call speakers attribute-segment [--commit] [--json] + sol call speakers backfill [--commit] [--json] sol call speakers discover [--json] sol call speakers identify [--entity-id ID] sol call speakers merge-names sol call speakers link-import --entity-id - sol call speakers seed-from-imports [--dry-run] [--json] + sol call speakers seed-from-imports [--commit] [--json] sol call speakers suggest [--limit N] [--json] - sol call speakers link-import --entity-id - sol call speakers seed-from-imports [--dry-run] [--json] sol call speakers detect [--json] sol call speakers confirm-owner [--backfill] [--json] sol call speakers reject-owner @@ -62,8 +64,10 @@ def status( @app.command("bootstrap") def bootstrap( - dry_run: bool = typer.Option( - False, "--dry-run", help="Show what would be saved without saving." + commit: bool = typer.Option( + False, + "--commit", + help="Persist results. Without this flag the command only reports what would happen.", ), json_output: bool = typer.Option( False, "--json", help="Output full result as JSON." @@ -78,12 +82,12 @@ def bootstrap( """ from apps.speakers.bootstrap import bootstrap_voiceprints - if dry_run and not json_output: - typer.echo("DRY RUN — no voiceprints will be saved\n") + if not commit and not json_output: + typer.echo("REPORT ONLY — pass --commit to persist.\n") if not json_output: typer.echo("Bootstrapping voiceprints from single-speaker segments...") - stats = bootstrap_voiceprints(dry_run=dry_run) + stats = bootstrap_voiceprints(dry_run=not commit) if "error" in stats: typer.echo(f"Error: {stats['error']}", err=True) @@ -122,8 +126,10 @@ def bootstrap( @app.command("resolve-names") def resolve_names( - dry_run: bool = typer.Option( - False, "--dry-run", help="Show merges without applying them." + commit: bool = typer.Option( + False, + "--commit", + help="Persist results. Without this flag the command only reports what would happen.", ), json_output: bool = typer.Option( False, "--json", help="Output full result as JSON." @@ -138,12 +144,12 @@ def resolve_names( """ from apps.speakers.bootstrap import resolve_name_variants - if dry_run and not json_output: - typer.echo("DRY RUN — no merges will be applied\n") + if not commit and not json_output: + typer.echo("REPORT ONLY — pass --commit to persist.\n") if not json_output: typer.echo("Resolving speaker name variants...") - stats = resolve_name_variants(dry_run=dry_run) + stats = resolve_name_variants(dry_run=not commit) if json_output: import json as json_mod @@ -181,6 +187,9 @@ def attribute_segment_cmd( day: str = typer.Argument(..., help="Day in YYYYMMDD format."), stream: str = typer.Argument(..., help="Stream name."), segment: str = typer.Argument(..., help="Segment key (HHMMSS_LEN)."), + commit: bool = typer.Option( + False, "--commit", help="Persist speaker labels and voiceprint accumulation." + ), save: bool = typer.Option( True, "--save/--no-save", help="Write speaker_labels.json." ), @@ -210,6 +219,9 @@ def attribute_segment_cmd( result = attribute_segment(day, stream, segment) + if not commit and not json_output: + typer.echo("REPORT ONLY — pass --commit to persist.\n") + if result.get("error"): typer.echo(f"Error: {result['error']}", err=True) raise typer.Exit(1) @@ -235,13 +247,13 @@ def attribute_segment_cmd( for method, count in sorted(methods.items()): typer.echo(f" {method}: {count}") - if save: + if commit and save: seg_dir = segment_path(day, segment, stream) out_path = save_speaker_labels(seg_dir, labels, metadata) if not json_output: typer.echo(f"\nWrote: {out_path}") - if accumulate and source: + if commit and accumulate and source: saved = accumulate_voiceprints(day, stream, segment, labels, source) if saved and not json_output: typer.echo("\nAccumulated voiceprints:") @@ -251,8 +263,10 @@ def attribute_segment_cmd( @app.command("backfill") def backfill( - dry_run: bool = typer.Option( - False, "--dry-run", help="Enumerate segments without processing." + commit: bool = typer.Option( + False, + "--commit", + help="Persist results. Without this flag the command only reports what would happen.", ), json_output: bool = typer.Option( False, "--json", help="Output full result as JSON." @@ -267,8 +281,8 @@ def backfill( from apps.speakers.attribution import backfill_segments - if dry_run and not json_output: - typer.echo("DRY RUN — no labels will be written\n") + if not commit and not json_output: + typer.echo("REPORT ONLY — pass --commit to persist.\n") if not json_output: typer.echo("Scanning journal for segments with embeddings...") @@ -288,8 +302,8 @@ def backfill( typer.echo(f" [{processed}/{total}]", nl=False) stats = backfill_segments( - dry_run=dry_run, - progress_callback=None if dry_run or json_output else on_progress, + dry_run=not commit, + progress_callback=None if not commit or json_output else on_progress, ) elapsed = time.monotonic() - start @@ -420,8 +434,10 @@ def link_import_cmd( @app.command("seed-from-imports") def seed_from_imports_cmd( - dry_run: bool = typer.Option( - False, "--dry-run", help="Show what would be saved without saving." + commit: bool = typer.Option( + False, + "--commit", + help="Persist results. Without this flag the command only reports what would happen.", ), json_output: bool = typer.Option( False, "--json", help="Output full result as JSON." @@ -436,12 +452,12 @@ def seed_from_imports_cmd( """ from apps.speakers.bootstrap import seed_from_imports - if dry_run and not json_output: - typer.echo("DRY RUN — no voiceprints will be saved\n") + if not commit and not json_output: + typer.echo("REPORT ONLY — pass --commit to persist.\n") if not json_output: typer.echo("Seeding voiceprints from import segments...") - stats = seed_from_imports(dry_run=dry_run) + stats = seed_from_imports(dry_run=not commit) if "error" in stats: typer.echo(f"Error: {stats['error']}", err=True) diff --git a/apps/speakers/tests/conftest.py b/apps/speakers/tests/conftest.py index 56514e9ce..32eaf1557 100644 --- a/apps/speakers/tests/conftest.py +++ b/apps/speakers/tests/conftest.py @@ -17,6 +17,12 @@ from think.entities import entity_slug STREAM = "test" +@pytest.fixture(autouse=True) +def _skip_supervisor_check(monkeypatch): + """Allow app CLI tests to run without a live solstone supervisor.""" + monkeypatch.setenv("SOL_SKIP_SUPERVISOR_CHECK", "1") + + @pytest.fixture def speakers_env(tmp_path, monkeypatch): """Create a temporary journal environment for speaker tests. diff --git a/apps/speakers/tests/test_call.py b/apps/speakers/tests/test_call.py new file mode 100644 index 000000000..fd12b9528 --- /dev/null +++ b/apps/speakers/tests/test_call.py @@ -0,0 +1,379 @@ +# SPDX-License-Identifier: AGPL-3.0-only +# Copyright (c) 2026 sol pbc + +"""CLI wrapper tests for speaker commands.""" + +from __future__ import annotations + +from typer.testing import CliRunner + +from apps.speakers.call import app as speakers_app + +runner = CliRunner() + + +def _bootstrap_stats() -> dict: + return { + "segments_scanned": 0, + "single_speaker_segments": 0, + "speakers_found": {}, + "entities_created": 0, + "embeddings_saved": 0, + "embeddings_skipped_owner": 0, + "embeddings_skipped_duplicate": 0, + "errors": [], + } + + +def _resolve_names_stats() -> dict: + return { + "entities_with_voiceprints": 0, + "pairs_compared": 0, + "matches_found": [], + "auto_merged": [], + "ambiguous": [], + "errors": [], + } + + +def _backfill_stats() -> dict: + return { + "total_segments": 0, + "total_eligible": 0, + "skipped_no_embed": 0, + "already_labeled": 0, + "processed": 0, + "speakers_seen": {}, + "errors": [], + } + + +def _seed_stats() -> dict: + return { + "segments_scanned": 0, + "segments_with_speakers": 0, + "speakers_found": {}, + "embeddings_saved": 0, + "embeddings_skipped_owner": 0, + "embeddings_skipped_duplicate": 0, + "speakers_unmatched": [], + "errors": [], + } + + +def _attribute_result() -> dict: + return { + "labels": [ + { + "sentence_id": 1, + "speaker": "alice", + "confidence": "high", + "method": "acoustic", + } + ], + "unmatched": [], + "source": "mic_audio", + "metadata": {}, + } + + +def test_bootstrap_default_is_preview(speakers_env, monkeypatch): + speakers_env() + seen: dict[str, bool] = {} + + def fake_bootstrap_voiceprints(*, dry_run: bool) -> dict: + seen["dry_run"] = dry_run + return _bootstrap_stats() + + monkeypatch.setattr( + "apps.speakers.bootstrap.bootstrap_voiceprints", + fake_bootstrap_voiceprints, + ) + + result = runner.invoke(speakers_app, ["bootstrap"]) + + assert result.exit_code == 0 + assert seen["dry_run"] is True + + +def test_bootstrap_commit_writes(speakers_env, monkeypatch): + speakers_env() + seen: dict[str, bool] = {} + + def fake_bootstrap_voiceprints(*, dry_run: bool) -> dict: + seen["dry_run"] = dry_run + return _bootstrap_stats() + + monkeypatch.setattr( + "apps.speakers.bootstrap.bootstrap_voiceprints", + fake_bootstrap_voiceprints, + ) + + result = runner.invoke(speakers_app, ["bootstrap", "--commit"]) + + assert result.exit_code == 0 + assert seen["dry_run"] is False + + +def test_resolve_names_default_is_preview(speakers_env, monkeypatch): + speakers_env() + seen: dict[str, bool] = {} + + def fake_resolve_name_variants(*, dry_run: bool) -> dict: + seen["dry_run"] = dry_run + return _resolve_names_stats() + + monkeypatch.setattr( + "apps.speakers.bootstrap.resolve_name_variants", + fake_resolve_name_variants, + ) + + result = runner.invoke(speakers_app, ["resolve-names"]) + + assert result.exit_code == 0 + assert seen["dry_run"] is True + + +def test_resolve_names_commit_writes(speakers_env, monkeypatch): + speakers_env() + seen: dict[str, bool] = {} + + def fake_resolve_name_variants(*, dry_run: bool) -> dict: + seen["dry_run"] = dry_run + return _resolve_names_stats() + + monkeypatch.setattr( + "apps.speakers.bootstrap.resolve_name_variants", + fake_resolve_name_variants, + ) + + result = runner.invoke(speakers_app, ["resolve-names", "--commit"]) + + assert result.exit_code == 0 + assert seen["dry_run"] is False + + +def test_backfill_default_is_preview(speakers_env, monkeypatch): + speakers_env() + seen: dict[str, bool] = {} + + def fake_backfill_segments(*, dry_run: bool, progress_callback=None) -> dict: + seen["dry_run"] = dry_run + return _backfill_stats() + + monkeypatch.setattr( + "apps.speakers.attribution.backfill_segments", + fake_backfill_segments, + ) + + result = runner.invoke(speakers_app, ["backfill"]) + + assert result.exit_code == 0 + assert seen["dry_run"] is True + + +def test_backfill_commit_writes(speakers_env, monkeypatch): + speakers_env() + seen: dict[str, bool] = {} + + def fake_backfill_segments(*, dry_run: bool, progress_callback=None) -> dict: + seen["dry_run"] = dry_run + return _backfill_stats() + + monkeypatch.setattr( + "apps.speakers.attribution.backfill_segments", + fake_backfill_segments, + ) + + result = runner.invoke(speakers_app, ["backfill", "--commit"]) + + assert result.exit_code == 0 + assert seen["dry_run"] is False + + +def test_seed_from_imports_default_is_preview(speakers_env, monkeypatch): + speakers_env() + seen: dict[str, bool] = {} + + def fake_seed_from_imports(*, dry_run: bool) -> dict: + seen["dry_run"] = dry_run + return _seed_stats() + + monkeypatch.setattr( + "apps.speakers.bootstrap.seed_from_imports", + fake_seed_from_imports, + ) + + result = runner.invoke(speakers_app, ["seed-from-imports"]) + + assert result.exit_code == 0 + assert seen["dry_run"] is True + + +def test_seed_from_imports_commit_writes(speakers_env, monkeypatch): + speakers_env() + seen: dict[str, bool] = {} + + def fake_seed_from_imports(*, dry_run: bool) -> dict: + seen["dry_run"] = dry_run + return _seed_stats() + + monkeypatch.setattr( + "apps.speakers.bootstrap.seed_from_imports", + fake_seed_from_imports, + ) + + result = runner.invoke(speakers_app, ["seed-from-imports", "--commit"]) + + assert result.exit_code == 0 + assert seen["dry_run"] is False + + +def test_attribute_segment_default_skips_writes(speakers_env, monkeypatch): + speakers_env() + save_calls: list[tuple] = [] + accumulate_calls: list[tuple] = [] + + monkeypatch.setattr( + "apps.speakers.attribution.attribute_segment", + lambda *_args, **_kwargs: _attribute_result(), + ) + monkeypatch.setattr( + "apps.speakers.attribution.save_speaker_labels", + lambda *args, **kwargs: save_calls.append((args, kwargs)), + ) + monkeypatch.setattr( + "apps.speakers.attribution.accumulate_voiceprints", + lambda *args, **kwargs: accumulate_calls.append((args, kwargs)), + ) + + result = runner.invoke( + speakers_app, + ["attribute-segment", "20240101", "test", "090000_300"], + ) + + assert result.exit_code == 0 + assert save_calls == [] + assert accumulate_calls == [] + + +def test_attribute_segment_commit_writes_both(speakers_env, monkeypatch, tmp_path): + speakers_env() + save_calls: list[tuple] = [] + accumulate_calls: list[tuple] = [] + + monkeypatch.setattr( + "apps.speakers.attribution.attribute_segment", + lambda *_args, **_kwargs: _attribute_result(), + ) + monkeypatch.setattr( + "apps.speakers.attribution.save_speaker_labels", + lambda *args, **kwargs: ( + save_calls.append((args, kwargs)) or (tmp_path / "speaker_labels.json") + ), + ) + monkeypatch.setattr( + "apps.speakers.attribution.accumulate_voiceprints", + lambda *args, **kwargs: accumulate_calls.append((args, kwargs)) or {"alice": 1}, + ) + monkeypatch.setattr("think.utils.segment_path", lambda *_args: tmp_path) + + result = runner.invoke( + speakers_app, + ["attribute-segment", "20240101", "test", "090000_300", "--commit"], + ) + + assert result.exit_code == 0 + assert len(save_calls) == 1 + assert len(accumulate_calls) == 1 + + +def test_attribute_segment_commit_no_save(speakers_env, monkeypatch, tmp_path): + speakers_env() + save_calls: list[tuple] = [] + accumulate_calls: list[tuple] = [] + + monkeypatch.setattr( + "apps.speakers.attribution.attribute_segment", + lambda *_args, **_kwargs: _attribute_result(), + ) + monkeypatch.setattr( + "apps.speakers.attribution.save_speaker_labels", + lambda *args, **kwargs: ( + save_calls.append((args, kwargs)) or (tmp_path / "speaker_labels.json") + ), + ) + monkeypatch.setattr( + "apps.speakers.attribution.accumulate_voiceprints", + lambda *args, **kwargs: accumulate_calls.append((args, kwargs)) or {"alice": 1}, + ) + monkeypatch.setattr("think.utils.segment_path", lambda *_args: tmp_path) + + result = runner.invoke( + speakers_app, + [ + "attribute-segment", + "20240101", + "test", + "090000_300", + "--commit", + "--no-save", + ], + ) + + assert result.exit_code == 0 + assert save_calls == [] + assert len(accumulate_calls) == 1 + + +def test_attribute_segment_commit_no_accumulate(speakers_env, monkeypatch, tmp_path): + speakers_env() + save_calls: list[tuple] = [] + accumulate_calls: list[tuple] = [] + + monkeypatch.setattr( + "apps.speakers.attribution.attribute_segment", + lambda *_args, **_kwargs: _attribute_result(), + ) + monkeypatch.setattr( + "apps.speakers.attribution.save_speaker_labels", + lambda *args, **kwargs: ( + save_calls.append((args, kwargs)) or (tmp_path / "speaker_labels.json") + ), + ) + monkeypatch.setattr( + "apps.speakers.attribution.accumulate_voiceprints", + lambda *args, **kwargs: accumulate_calls.append((args, kwargs)) or {"alice": 1}, + ) + monkeypatch.setattr("think.utils.segment_path", lambda *_args: tmp_path) + + result = runner.invoke( + speakers_app, + [ + "attribute-segment", + "20240101", + "test", + "090000_300", + "--commit", + "--no-accumulate", + ], + ) + + assert result.exit_code == 0 + assert len(save_calls) == 1 + assert accumulate_calls == [] + + +def test_speaker_commands_reject_removed_dry_run_flag(speakers_env): + speakers_env() + commands = [ + ["bootstrap", "--dry-run"], + ["resolve-names", "--dry-run"], + ["backfill", "--dry-run"], + ["seed-from-imports", "--dry-run"], + ["attribute-segment", "20240101", "test", "090000_300", "--dry-run"], + ] + + for argv in commands: + result = runner.invoke(speakers_app, argv) + assert result.exit_code != 0 diff --git a/apps/speakers/tests/test_import_composition.py b/apps/speakers/tests/test_import_composition.py index 8c8ed664f..876b74aa2 100644 --- a/apps/speakers/tests/test_import_composition.py +++ b/apps/speakers/tests/test_import_composition.py @@ -227,7 +227,7 @@ def test_seed_from_imports_cli_json_success(speakers_env, monkeypatch): ) _mock_owner(monkeypatch, _normalized_vector(22), threshold=0.99) - result = _runner.invoke(speakers_app, ["seed-from-imports", "--json"]) + result = _runner.invoke(speakers_app, ["seed-from-imports", "--commit", "--json"]) assert result.exit_code == 0 data = json.loads(result.output) diff --git a/apps/speakers/tests/test_seed_imports.py b/apps/speakers/tests/test_seed_imports.py index d2b5590ae..2949e2769 100644 --- a/apps/speakers/tests/test_seed_imports.py +++ b/apps/speakers/tests/test_seed_imports.py @@ -131,7 +131,7 @@ def test_seed_from_imports_happy_path(speakers_env): embeddings=embs, ) - result = _runner.invoke(speakers_app, ["seed-from-imports", "--json"]) + result = _runner.invoke(speakers_app, ["seed-from-imports", "--commit", "--json"]) assert result.exit_code == 0, result.output data = json.loads(result.output) assert data["segments_scanned"] >= 1 @@ -230,21 +230,21 @@ def test_seed_from_imports_dedup(speakers_env): ) # First run - result1 = _runner.invoke(speakers_app, ["seed-from-imports", "--json"]) + result1 = _runner.invoke(speakers_app, ["seed-from-imports", "--commit", "--json"]) assert result1.exit_code == 0 data1 = json.loads(result1.output) assert data1["embeddings_saved"] == 1 # Second run — should be all duplicates - result2 = _runner.invoke(speakers_app, ["seed-from-imports", "--json"]) + result2 = _runner.invoke(speakers_app, ["seed-from-imports", "--commit", "--json"]) assert result2.exit_code == 0 data2 = json.loads(result2.output) assert data2["embeddings_saved"] == 0 assert data2["embeddings_skipped_duplicate"] == 1 -def test_seed_from_imports_dry_run(speakers_env): - """seed-from-imports --dry-run reports stats but doesn't write.""" +def test_seed_from_imports_default_is_preview(speakers_env): + """seed-from-imports defaults to preview mode and doesn't write.""" env = speakers_env() _create_owner_centroid(env) env.create_entity("Alice Johnson") @@ -261,7 +261,7 @@ def test_seed_from_imports_dry_run(speakers_env): result = _runner.invoke( speakers_app, - ["seed-from-imports", "--dry-run", "--json"], + ["seed-from-imports", "--json"], ) assert result.exit_code == 0 data = json.loads(result.output) diff --git a/apps/todos/call.py b/apps/todos/call.py index 0fe53616a..8fee01021 100644 --- a/apps/todos/call.py +++ b/apps/todos/call.py @@ -6,6 +6,9 @@ Auto-discovered by ``think.call`` and mounted as ``sol call todos ...``. """ +import json +import subprocess +from datetime import datetime from pathlib import Path import typer @@ -398,20 +401,10 @@ def upcoming_todos( typer.echo(result) -@app.command("check-nudges") -def check_nudges( - facet: str | None = typer.Option( - None, - "--facet", - "-f", - help="Facet name (or set SOL_FACET). Omit to check all facets.", - ), -) -> None: - """Check for un-notified past nudges and send notifications.""" - import subprocess - from datetime import datetime - from pathlib import Path - +def _due_nudges( + facet: str | None, +) -> list[tuple[str, todo.TodoChecklist, todo.TodoItem]]: + """Return due, unnotified nudges for today without mutating state.""" from think.utils import get_journal, resolve_sol_facet journal = get_journal() @@ -420,7 +413,7 @@ def check_nudges( facets_dir = Path(journal) / "facets" if not facets_dir.is_dir(): - return + return [] if facet is not None: facet = resolve_sol_facet(facet) @@ -428,12 +421,11 @@ def check_nudges( else: facet_names = [d.name for d in facets_dir.iterdir() if d.is_dir()] + due: list[tuple[str, todo.TodoChecklist, todo.TodoItem]] = [] for facet_name in facet_names: checklist = todo.TodoChecklist.load(today, facet_name) if not checklist.exists: continue - - modified = False for item in checklist.items: if ( item.nudge @@ -442,32 +434,102 @@ def check_nudges( and not item.completed and not item.cancelled ): - # Send notification via sol notify - try: - subprocess.run( - [ - "sol", - "notify", - item.text, - "--title", - "Todo Reminder", - "--icon", - "✅", - "--app", - "todos", - "--facet", - facet_name, - "--action", - f"/app/todos/{today}", - ], - check=False, - capture_output=True, - ) - except FileNotFoundError: - pass # sol not available - item.notified = True - modified = True - typer.echo(f"Notified: [{facet_name}] {item.text}") - - if modified: - checklist.save() + due.append((facet_name, checklist, item)) + return due + + +@app.command("list-nudges-due") +def list_nudges_due( + facet: str | None = typer.Option( + None, + "--facet", + "-f", + help="Facet name (or set SOL_FACET). Omit to check all facets.", + ), + json_output: bool = typer.Option(False, "--json", help="Output as JSON."), +) -> None: + """List due, unnotified todo nudges.""" + due = _due_nudges(facet) + now = datetime.now() + if json_output: + payload = [ + { + "day": datetime.now().strftime("%Y%m%d"), + "facet": facet_name, + "index": item.index, + "text": item.text, + "nudge": item.nudge, + "nudge_display": todo.format_nudge(item.nudge or "", now=now), + } + for facet_name, _checklist, item in due + ] + typer.echo(json.dumps(payload, ensure_ascii=False, indent=2)) + return + + if not due: + typer.echo("No nudges due.") + return + + grouped: dict[str, list[todo.TodoItem]] = {} + for facet_name, _checklist, item in due: + grouped.setdefault(facet_name, []).append(item) + + if len(grouped) == 1: + items = next(iter(grouped.values())) + for item in items: + typer.echo(f"{item.index}: {item.display_line()}") + return + + for facet_name, items in grouped.items(): + typer.echo(f"## {facet_name}") + for item in items: + typer.echo(f"{item.index}: {item.display_line()}") + typer.echo() + + +@app.command("dispatch-nudges") +def dispatch_nudges( + facet: str | None = typer.Option( + None, + "--facet", + "-f", + help="Facet name (or set SOL_FACET). Omit to check all facets.", + ), +) -> None: + """Dispatch due, unnotified todo nudges.""" + due = _due_nudges(facet) + today = datetime.now().strftime("%Y%m%d") + modified_checklists: dict[str, todo.TodoChecklist] = {} + dispatched = 0 + + for facet_name, checklist, item in due: + try: + subprocess.run( + [ + "sol", + "notify", + item.text, + "--title", + "Todo Reminder", + "--icon", + "✅", + "--app", + "todos", + "--facet", + facet_name, + "--action", + f"/app/todos/{today}", + ], + check=False, + capture_output=True, + ) + except FileNotFoundError: + pass + item.notified = True + modified_checklists[facet_name] = checklist + dispatched += 1 + + for checklist in modified_checklists.values(): + checklist.save() + + typer.echo(f"dispatched {dispatched} nudge(s)") diff --git a/apps/todos/tests/conftest.py b/apps/todos/tests/conftest.py index e0310010a..18dcb4111 100644 --- a/apps/todos/tests/conftest.py +++ b/apps/todos/tests/conftest.py @@ -15,6 +15,12 @@ from datetime import datetime import pytest +@pytest.fixture(autouse=True) +def _skip_supervisor_check(monkeypatch): + """Allow app CLI tests to run without a live solstone supervisor.""" + monkeypatch.setenv("SOL_SKIP_SUPERVISOR_CHECK", "1") + + @pytest.fixture def todo_env(tmp_path, monkeypatch): """Create a temporary journal facet with optional todo entries. diff --git a/apps/todos/tests/test_call.py b/apps/todos/tests/test_call.py index 5e0f51f77..90a1acbfb 100644 --- a/apps/todos/tests/test_call.py +++ b/apps/todos/tests/test_call.py @@ -4,9 +4,11 @@ """Tests for todos CLI commands (sol call todos ...).""" import json +from datetime import datetime from typer.testing import CliRunner +import apps.todos.call as todos_call from think.call import call_app runner = CliRunner() @@ -199,6 +201,149 @@ class TestTodosUpcoming: assert "No upcoming todos" in result.output +class TestTodosNudges: + class _FixedDateTime: + @classmethod + def now(cls): + return datetime(2026, 3, 10, 12, 0) + + def test_list_nudges_due_is_readonly(self, todo_env, monkeypatch): + day, facet, todo_path = todo_env( + [{"text": "Follow up", "nudge": "20260310T09:00"}], + day="20260310", + ) + before = todo_path.read_text(encoding="utf-8") + monkeypatch.setattr(todos_call, "datetime", self._FixedDateTime) + + result = runner.invoke(call_app, ["todos", "list-nudges-due", "--facet", facet]) + + assert result.exit_code == 0 + assert "Follow up" in result.output + assert todo_path.read_text(encoding="utf-8") == before + + def test_list_nudges_due_json_all_facets(self, todo_env, monkeypatch): + todo_env( + [{"text": "Work ping", "nudge": "20260310T08:00"}], + day="20260310", + facet="work", + ) + todo_env( + [{"text": "Home ping", "nudge": "20260310T09:00"}], + day="20260310", + facet="home", + ) + monkeypatch.setattr(todos_call, "datetime", self._FixedDateTime) + + result = runner.invoke(call_app, ["todos", "list-nudges-due", "--json"]) + + assert result.exit_code == 0 + payload = json.loads(result.output) + assert payload == [ + { + "day": "20260310", + "facet": "work", + "index": 1, + "text": "Work ping", + "nudge": "20260310T08:00", + "nudge_display": "4h ago", + }, + { + "day": "20260310", + "facet": "home", + "index": 1, + "text": "Home ping", + "nudge": "20260310T09:00", + "nudge_display": "3h ago", + }, + ] + + def test_list_nudges_due_empty(self, todo_env, monkeypatch): + todo_env([], day="20260310") + monkeypatch.setattr(todos_call, "datetime", self._FixedDateTime) + + human = runner.invoke(call_app, ["todos", "list-nudges-due"]) + json_result = runner.invoke(call_app, ["todos", "list-nudges-due", "--json"]) + + assert human.exit_code == 0 + assert human.output.strip() == "No nudges due." + assert json_result.exit_code == 0 + assert json.loads(json_result.output) == [] + + def test_dispatch_nudges_notifies_and_marks(self, todo_env, monkeypatch): + _day, facet, todo_path = todo_env( + [{"text": "Follow up", "nudge": "20260310T09:00"}], + day="20260310", + ) + calls: list[tuple[list[str], dict]] = [] + + def fake_run(argv, **kwargs): + calls.append((argv, kwargs)) + return None + + monkeypatch.setattr(todos_call, "datetime", self._FixedDateTime) + monkeypatch.setattr(todos_call.subprocess, "run", fake_run) + + result = runner.invoke(call_app, ["todos", "dispatch-nudges", "--facet", facet]) + + assert result.exit_code == 0 + assert result.output.strip() == "dispatched 1 nudge(s)" + assert calls == [ + ( + [ + "sol", + "notify", + "Follow up", + "--title", + "Todo Reminder", + "--icon", + "✅", + "--app", + "todos", + "--facet", + facet, + "--action", + "/app/todos/20260310", + ], + {"check": False, "capture_output": True}, + ) + ] + saved = [ + json.loads(line) + for line in todo_path.read_text(encoding="utf-8").splitlines() + ] + assert saved == [ + { + "text": "Follow up", + "nudge": "20260310T09:00", + "notified": True, + } + ] + + def test_dispatch_nudges_noop_when_nothing_due(self, todo_env, monkeypatch): + todo_env([], day="20260310") + calls: list[tuple[list[str], dict]] = [] + + def fake_run(argv, **kwargs): + calls.append((argv, kwargs)) + return None + + monkeypatch.setattr(todos_call, "datetime", self._FixedDateTime) + monkeypatch.setattr(todos_call.subprocess, "run", fake_run) + + result = runner.invoke(call_app, ["todos", "dispatch-nudges"]) + + assert result.exit_code == 0 + assert result.output.strip() == "dispatched 0 nudge(s)" + assert calls == [] + + def test_legacy_nudge_command_removed(self, todo_env): + todo_env([], day="20260310") + + result = runner.invoke(call_app, ["todos", "check" + "-nudges"]) + + assert result.exit_code != 0 + + class TestTodosMove: """Tests for 'sol call todos move' command.""" diff --git a/docs/SOLCLI.md b/docs/SOLCLI.md index 1ab03efc1..cdaab7ad6 100644 --- a/docs/SOLCLI.md +++ b/docs/SOLCLI.md @@ -246,7 +246,7 @@ This is for audit trail — it records that the agent confirmed user consent bef @app.command("create") # CRUD verbs ``` -Use lowercase, single-word names. Hyphenated names for multi-word (`check-nudges`, `set-name`). +Use lowercase, single-word names. Hyphenated names for multi-word (`list-nudges-due`, `set-name`). ## Directory Structure @@ -305,7 +305,7 @@ solstone/ | App | Source | Commands | |-----|--------|----------| -| `todos` | `apps/todos/call.py` | list, add, done, cancel, move, upcoming, check-nudges | +| `todos` | `apps/todos/call.py` | list, add, done, cancel, move, upcoming, list-nudges-due, dispatch-nudges | | `calendar` | `apps/calendar/call.py` | list, add, update, cancel, move, import-day | | `entities` | `apps/entities/call.py` | list, show, search, observe, merge | | `speakers` | `apps/speakers/call.py` | list, show, detect-owner, confirm-owner, clusters, suggest | diff --git a/scripts/check_layer_hygiene.py b/scripts/check_layer_hygiene.py index a8abc084f..f48f21b64 100644 --- a/scripts/check_layer_hygiene.py +++ b/scripts/check_layer_hygiene.py @@ -71,9 +71,8 @@ TARGET_PATH_PATTERNS: tuple[re.Pattern[str], ...] = ( ), ) -# Read verbs per docs/coding-standards.md § L3. Matched against any -# underscore-split segment of the function name, so ``keys_validate`` and -# ``check_nudges`` both trip the rule. +# Read verbs per docs/coding-standards.md § L3. Match against any +# underscore-split segment of the function name. READ_VERBS: frozenset[str] = frozenset( { "load", @@ -103,20 +102,10 @@ READ_VERBS: frozenset[str] = frozenset( } ) -# Known violations from the solstone layer-violations audit (2026-04-17). -# Each entry silences the lint for an entire file until the underlying -# violation is fixed. Remove the entry when its bundle ships. -# -# Audit ref: vpe/workspace/solstone-layer-violations-audit.md (extro repo). -ALLOWLIST: dict[str, str] = { - # TODO(import-resolve-facet): apps/import/call.py's `resolve-facet` - # command uses a read-verb name ("resolve_*" per L3) but writes to - # journal/facets and unlinks staged files. Not in the audit's V1-V14, - # but surfaced by this lint on first run. Needs CPO/VPE disposition: - # rename to a write verb (e.g. `apply-staged-facet` + `skip-staged-facet`) - # or accept as a V13-class dual-mode verb. - "apps/import/call.py": "import-resolve-facet", -} +# Temporary, file-scoped exceptions for known layer-hygiene violations. +# Keep this empty by default; add entries only with a tracking identifier +# and remove them in the same bundle that fixes the violation. +ALLOWLIST: dict[str, str] = {} CONTEXT_WINDOW = 8 # lines above and below each write to search for paths diff --git a/tests/test_import_call.py b/tests/test_import_call.py index 40f75eb5e..ba80bba3e 100644 --- a/tests/test_import_call.py +++ b/tests/test_import_call.py @@ -618,7 +618,14 @@ def test_resolve_facet_apply_unmapped_entity(import_env): result = runner.invoke( call_app, - ["import", "resolve-facet", staged_file, "apply", "--source", "test-source"], + [ + "import", + "resolve-staged-facet", + staged_file, + "--apply", + "--source", + "test-source", + ], ) assert result.exit_code == 0 @@ -657,7 +664,14 @@ def test_resolve_facet_apply_facet_json_conflict(import_env): result = runner.invoke( call_app, - ["import", "resolve-facet", staged_file, "apply", "--source", "test-source"], + [ + "import", + "resolve-staged-facet", + staged_file, + "--apply", + "--source", + "test-source", + ], ) assert result.exit_code == 0 @@ -699,7 +713,14 @@ def test_resolve_facet_unmapped_entity_fails_without_mapping(import_env): result = runner.invoke( call_app, - ["import", "resolve-facet", staged_file, "apply", "--source", "test-source"], + [ + "import", + "resolve-staged-facet", + staged_file, + "--apply", + "--source", + "test-source", + ], ) assert result.exit_code == 1 @@ -737,7 +758,14 @@ def test_resolve_facet_skip(import_env): result = runner.invoke( call_app, - ["import", "resolve-facet", staged_file, "skip", "--source", "test-source"], + [ + "import", + "resolve-staged-facet", + staged_file, + "--skip", + "--source", + "test-source", + ], ) assert result.exit_code == 0 @@ -745,6 +773,85 @@ def test_resolve_facet_skip(import_env): assert load_facet_relationship("personal", "source_entity") is None +def test_resolve_staged_facet_requires_exactly_one_mode_flag(import_env): + staged_file = "personal/facet_json/facet.json.staged.json" + staged_path = ( + get_state_directory(import_env["key_prefix"]) + / "facets" + / "staged" + / staged_file + ) + _write_json( + staged_path, + { + "reason": "facet_json_conflict", + "source_content": {"title": "Remote"}, + "target_content": {"title": "Local"}, + "staged_at": "2026-04-14T00:00:00+00:00", + }, + ) + + result = runner.invoke( + call_app, + ["import", "resolve-staged-facet", staged_file, "--source", "test-source"], + ) + + assert result.exit_code == 1 + assert "Exactly one of --apply or --skip is required." in result.stderr + + +def test_resolve_staged_facet_rejects_both_apply_and_skip(import_env): + staged_file = "personal/facet_json/facet.json.staged.json" + staged_path = ( + get_state_directory(import_env["key_prefix"]) + / "facets" + / "staged" + / staged_file + ) + _write_json( + staged_path, + { + "reason": "facet_json_conflict", + "source_content": {"title": "Remote"}, + "target_content": {"title": "Local"}, + "staged_at": "2026-04-14T00:00:00+00:00", + }, + ) + + result = runner.invoke( + call_app, + [ + "import", + "resolve-staged-facet", + staged_file, + "--apply", + "--skip", + "--source", + "test-source", + ], + ) + + assert result.exit_code == 1 + assert "Exactly one of --apply or --skip is required." in result.stderr + + +def test_resolve_facet_command_removed(import_env): + legacy_command = "resolve" + "-facet" + result = runner.invoke( + call_app, + [ + "import", + legacy_command, + "personal/facet_json/facet.json.staged.json", + "--apply", + "--source", + "test-source", + ], + ) + + assert result.exit_code != 0 + + def test_resolve_source_not_found(import_env): result = runner.invoke( call_app,