diff --git a/README.md b/README.md index 3e5e4c2..4354aef 100644 --- a/README.md +++ b/README.md @@ -242,7 +242,15 @@ hooks: type: llmlingua ``` -See [protocols/secom/README.md](src/fava_trails/protocols/secom/README.md) for full config reference and model options. See [AGENTS_SETUP_INSTRUCTIONS.md](AGENTS_SETUP_INSTRUCTIONS.md#lifecycle-hooks) for the general hooks system. +**Structured data**: SECOM's token-level compression has no notion of syntactic validity — JSON objects, YAML blocks, and fenced code blocks may be silently destroyed at promote time. Tag thoughts with `secom-skip` to opt out: + +```python +save_thought(trail_name="my/scope", content='{"phases": [...]}', metadata={"tags": ["secom-skip"]}) +``` + +The `before_save` hook warns when structured content is detected without `secom-skip`. + +See [protocols/secom/README.md](src/fava_trails/protocols/secom/README.md) for full config reference, model options, and the `secom-skip` opt-out. See [AGENTS_SETUP_INSTRUCTIONS.md](AGENTS_SETUP_INSTRUCTIONS.md#lifecycle-hooks) for the general hooks system. ## Development diff --git a/docs/fava_trails_faq.md b/docs/fava_trails_faq.md index 86bf992..74dd644 100755 --- a/docs/fava_trails_faq.md +++ b/docs/fava_trails_faq.md @@ -45,6 +45,8 @@ For example, the built-in [SECOM protocol](../src/fava_trails/protocols/secom/RE Install with `pip install fava-trails[secom]` and add a `hooks:` entry to your data repo's `config.yaml`. See the [Protocols section](../README.md#protocols) for quick start. +**Known limitation**: SECOM's extractive token-level compression operates at the token level and has no notion of syntactic structure. JSON objects, YAML blocks, and fenced code blocks can be silently destroyed at promote time. Use the `secom-skip` tag to opt out of compression for any thought containing structured data — the `before_save` hook will warn you when it detects structured content without this tag. + --- ## The Architecture diff --git a/src/fava_trails/protocols/secom/README.md b/src/fava_trails/protocols/secom/README.md index 6f5468a..fa74c29 100644 --- a/src/fava_trails/protocols/secom/README.md +++ b/src/fava_trails/protocols/secom/README.md @@ -127,15 +127,43 @@ pip install fava-trails[secom] # or: uv add fava-trails[secom] ``` +## Structured Data + +SECOM's token-level compression has no notion of syntactic validity — JSON objects, YAML blocks, and fenced code blocks may be silently destroyed at promote time. + +### Opt Out with `secom-skip` + +Tag any thought with `secom-skip` to bypass compression entirely: + +```python +save_thought( + trail_name="my/scope", + content='{"phases": ["build", "test"]}', + metadata={"tags": ["secom-skip"]}, +) +``` + +`secom-skip` means "do not compress this." It is semantically distinct from `secom-compressed` (which means "already compressed"). + +### Advisory on Save + +When `before_save` detects structured content (fenced code blocks or JSON-like lines) and `secom-skip` is absent, it issues an `Advise` action with code `secom_structured_data_advisory`, suggesting you add the tag before promoting. + +### Operator Warning on Compress + +If compression proceeds on content that appears structured, `before_propose` logs a `WARNING` so operators can investigate unexpected data corruption. + ## Hooks ### `before_propose` -- Inline Compression -Compresses content via `Mutate(ThoughtPatch)` before promotion. Adds `secom-compressed` tag and compression metadata to `extra`. Skips if content is below threshold or already compressed. Fails open on errors. +Compresses content via `Mutate(ThoughtPatch)` before promotion. Adds `secom-compressed` tag and compression metadata to `extra`. Skips if content is below threshold, already tagged `secom-compressed`, or explicitly tagged `secom-skip`. Logs a `WARNING` when compressing content with detected structured data. Fails open on errors. ### `before_save` -- Verbosity Advisor -Issues an `Advise` when saved content exceeds `verbosity_warn_chars`, suggesting front-loading key facts since extractive compression preserves tokens in order. +Issues `Advise` actions when: +- Saved content exceeds `verbosity_warn_chars` (code: `secom_verbosity_advisory`) — suggests front-loading key facts +- Content contains structured data (JSON/YAML/code blocks) and `secom-skip` is absent (code: `secom_structured_data_advisory`) — suggests adding the `secom-skip` tag ### `on_recall` -- Density-Aware Scoring diff --git a/src/fava_trails/protocols/secom/__init__.py b/src/fava_trails/protocols/secom/__init__.py index 87fc7b7..06bbb6a 100644 --- a/src/fava_trails/protocols/secom/__init__.py +++ b/src/fava_trails/protocols/secom/__init__.py @@ -36,6 +36,7 @@ from __future__ import annotations import logging +import re from typing import Any from fava_trails.hook_types import ( @@ -158,6 +159,23 @@ def _compress(text: str, target_rate: float) -> tuple[str, float]: return compressed, actual_rate +# --- Structured Data Detection --- + + +def _has_structured_data(content: str) -> bool: + """Return True if content appears to contain structured data. + + Heuristics (any one triggers): + - Fenced code block (``` ... ```) + - JSON-like block: line starting with { or [ + """ + if re.search(r"^```", content, re.MULTILINE): + return True + if re.search(r"^\s*[{\[]", content, re.MULTILINE): + return True + return False + + # --- Lifecycle Hooks --- @@ -177,14 +195,21 @@ async def before_propose(event: BeforeProposeEvent) -> list[Any] | None: if len(content.strip()) < threshold: return None - # Don't re-compress + # Explicit opt-out or already compressed tags = event.thought.frontmatter.metadata.tags or [] - if "secom-compressed" in tags: + if "secom-skip" in tags or "secom-compressed" in tags: return None target_rate = _CONFIG.get("target_compress_rate", 0.6) engine_type = _ENGINE_CONFIG.get("type", "llmlingua") + if _has_structured_data(content): + logger.warning( + "SECOM: compressing content with detected structured data " + "(JSON/YAML/code block) — syntactic validity may be lost. " + "Add 'secom-skip' tag to preserve structure." + ) + try: compressed, actual_rate = _compress(content, target_rate) except Exception as e: @@ -220,27 +245,38 @@ async def before_propose(event: BeforeProposeEvent) -> list[Any] | None: async def before_save(event: BeforeSaveEvent) -> list[Any] | None: - """Advise on verbose thoughts with front-loading guidance.""" + """Advise on verbose thoughts and structured content that may be harmed by compression.""" if not event.thought: return None content = event.thought.content - warn_chars = _CONFIG.get("verbosity_warn_chars", 1000) - - if len(content.strip()) < warn_chars: - return None + tags = event.thought.frontmatter.metadata.tags or [] + actions: list[Any] = [] - threshold = _CONFIG.get("compression_threshold_chars", 500) + warn_chars = _CONFIG.get("verbosity_warn_chars", 1000) + if len(content.strip()) >= warn_chars: + threshold = _CONFIG.get("compression_threshold_chars", 500) + actions.append(Advise( + message=( + f"Thought is {len(content)} chars. Content above {threshold} chars " + f"will be compressed at promote time (SECOM \u03c4={_CONFIG.get('target_compress_rate', 0.6)}). " + "Consider front-loading key facts and identifiers -- extractive compression " + "preserves tokens in order, so leading content survives at higher rates." + ), + code="secom_verbosity_advisory", + )) + + if "secom-skip" not in tags and _has_structured_data(content): + actions.append(Advise( + message=( + "Content appears to contain structured data (JSON/YAML/code block). " + "SECOM's extractive compression may destroy syntactic validity at promote time. " + "Add the 'secom-skip' tag to opt out of compression for this thought." + ), + code="secom_structured_data_advisory", + )) - return [Advise( - message=( - f"Thought is {len(content)} chars. Content above {threshold} chars " - f"will be compressed at promote time (SECOM \u03c4={_CONFIG.get('target_compress_rate', 0.6)}). " - "Consider front-loading key facts and identifiers -- extractive compression " - "preserves tokens in order, so leading content survives at higher rates." - ), - code="secom_verbosity_advisory", - )] + return actions or None async def on_recall(event: OnRecallEvent) -> list[Any] | None: diff --git a/tests/test_secom.py b/tests/test_secom.py index d7cd900..5805406 100644 --- a/tests/test_secom.py +++ b/tests/test_secom.py @@ -7,6 +7,7 @@ from __future__ import annotations +import logging from unittest.mock import patch import pytest @@ -244,6 +245,33 @@ async def test_idempotency_no_double_compress(self): result = await secom.before_propose(event) assert result is None + @pytest.mark.asyncio + async def test_secom_skip_tag_bypasses_compression(self): + """Thoughts tagged secom-skip are not compressed.""" + _configure(compression_threshold_chars=10) + thought = _make_thought( + content="A" * 1000, + tags=["secom-skip"], + ) + event = BeforeProposeEvent(trail_name="t", thought=thought) + result = await secom.before_propose(event) + assert result is None + + @pytest.mark.asyncio + async def test_structured_data_warning_logged(self, caplog): + """Compressing content with structured data emits a WARNING log.""" + _configure(compression_threshold_chars=10) + content = "Plan:\n```json\n{\"phases\": []}\n```\n" + "x" * 200 + + with patch("fava_trails.protocols.secom._compress", side_effect=_mock_compress): + thought = _make_thought(content=content) + event = BeforeProposeEvent(trail_name="t", thought=thought) + with caplog.at_level(logging.WARNING, logger="fava_trails.protocols.secom"): + result = await secom.before_propose(event) + + assert result is not None + assert any("structured data" in r.message for r in caplog.records if r.levelno == logging.WARNING) + @pytest.mark.asyncio async def test_preserves_existing_tags(self): """Compression adds secom-compressed without removing existing tags.""" @@ -295,6 +323,68 @@ async def test_no_thought_returns_none(self): result = await secom.before_save(event) assert result is None + @pytest.mark.asyncio + async def test_structured_data_fenced_code_block_advises(self): + """Content with fenced code block triggers structured data advisory.""" + _configure(verbosity_warn_chars=10000) + content = "Some thought.\n```json\n{\"key\": \"value\"}\n```\n" + thought = _make_thought(content=content) + event = BeforeSaveEvent(trail_name="t", thought=thought) + result = await secom.before_save(event) + assert result is not None + codes = [a.code for a in result if isinstance(a, Advise)] + assert "secom_structured_data_advisory" in codes + + @pytest.mark.asyncio + async def test_structured_data_json_block_advises(self): + """Content with a JSON-like line triggers structured data advisory.""" + _configure(verbosity_warn_chars=10000) + content = "Plan:\n{\n \"phases\": [\"a\", \"b\"]\n}\n" + thought = _make_thought(content=content) + event = BeforeSaveEvent(trail_name="t", thought=thought) + result = await secom.before_save(event) + assert result is not None + codes = [a.code for a in result if isinstance(a, Advise)] + assert "secom_structured_data_advisory" in codes + + @pytest.mark.asyncio + async def test_structured_data_advisory_suppressed_with_secom_skip(self): + """secom-skip tag suppresses structured data advisory.""" + _configure(verbosity_warn_chars=10000) + content = "Some thought.\n```json\n{\"key\": \"value\"}\n```\n" + thought = _make_thought(content=content, tags=["secom-skip"]) + event = BeforeSaveEvent(trail_name="t", thought=thought) + result = await secom.before_save(event) + codes = [a.code for a in result if isinstance(a, Advise)] if result else [] + assert "secom_structured_data_advisory" not in codes + + @pytest.mark.asyncio + async def test_both_advisories_triggered(self): + """Content with verbosity and structured data triggers both advisories.""" + _configure(verbosity_warn_chars=50, compression_threshold_chars=30) + content = "A" * 100 + "\n```json\n{\"key\": \"value\"}\n```" + thought = _make_thought(content=content) + event = BeforeSaveEvent(trail_name="t", thought=thought) + result = await secom.before_save(event) + assert result is not None + assert len(result) == 2 + codes = [a.code for a in result if isinstance(a, Advise)] + assert "secom_verbosity_advisory" in codes + assert "secom_structured_data_advisory" in codes + + @pytest.mark.asyncio + async def test_secom_skip_does_not_suppress_verbosity_advisory(self): + """secom-skip suppresses only structured data advisory, not verbosity advisory.""" + _configure(verbosity_warn_chars=50, compression_threshold_chars=30) + content = "A" * 100 + "\n```json\n{\"key\": \"value\"}\n```" + thought = _make_thought(content=content, tags=["secom-skip"]) + event = BeforeSaveEvent(trail_name="t", thought=thought) + result = await secom.before_save(event) + assert result is not None + codes = [a.code for a in result if isinstance(a, Advise)] + assert "secom_verbosity_advisory" in codes + assert "secom_structured_data_advisory" not in codes + # --- on_recall Hook ---