From 3768dd568a0dbc1550562e1f1ea3f4e0763c45a6 Mon Sep 17 00:00:00 2001 From: "Younes (Machine Wisdom)" Date: Tue, 10 Mar 2026 08:06:10 -0400 Subject: [PATCH 1/2] fix(secom): add secom-skip opt-out tag and structured-data safety warnings MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Closes spec 20. Three-part fix for silent JSON/YAML destruction at promote time: 1. secom-skip tag — explicit opt-out bypass in before_propose (semantically honest alternative to the secom-compressed workaround) 2. before_save advisory — warns when fenced code blocks or JSON-like lines are detected and secom-skip is absent (code: secom_structured_data_advisory) 3. before_propose WARNING log — surfaces structured-data compression to operators so silent data corruption can be investigated Existing secom-compressed bypass and all prior tests continue to pass. Co-Authored-By: Claude Sonnet 4.6 --- src/fava_trails/protocols/secom/README.md | 32 +++++++++- src/fava_trails/protocols/secom/__init__.py | 70 ++++++++++++++++----- tests/test_secom.py | 63 +++++++++++++++++++ 3 files changed, 146 insertions(+), 19 deletions(-) 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..fb32d90 100644 --- a/tests/test_secom.py +++ b/tests/test_secom.py @@ -244,6 +244,34 @@ 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.""" + import logging + _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,41 @@ 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 + # --- on_recall Hook --- From d1e4f20e9e4a72fbc9aa74b8e45766e76a584a8f Mon Sep 17 00:00:00 2001 From: "Younes (Machine Wisdom)" Date: Tue, 10 Mar 2026 08:08:57 -0400 Subject: [PATCH 2/2] test(secom): address devstral review gaps; docs: add known limitation notes From devstral code review: - Move `import logging` to module top in test file - Add test_both_advisories_triggered: verbose + structured content yields both Advise actions - Add test_secom_skip_does_not_suppress_verbosity_advisory: secom-skip is scoped to structured data only Docs: - README.md: add secom-skip structured data note and known limitation to SECOM section - docs/fava_trails_faq.md: add known limitation paragraph to SECOM FAQ answer Co-Authored-By: Claude Sonnet 4.6 --- README.md | 10 +++++++++- docs/fava_trails_faq.md | 2 ++ tests/test_secom.py | 29 ++++++++++++++++++++++++++++- 3 files changed, 39 insertions(+), 2 deletions(-) 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/tests/test_secom.py b/tests/test_secom.py index fb32d90..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 @@ -259,7 +260,6 @@ async def test_secom_skip_tag_bypasses_compression(self): @pytest.mark.asyncio async def test_structured_data_warning_logged(self, caplog): """Compressing content with structured data emits a WARNING log.""" - import logging _configure(compression_threshold_chars=10) content = "Plan:\n```json\n{\"phases\": []}\n```\n" + "x" * 200 @@ -358,6 +358,33 @@ async def test_structured_data_advisory_suppressed_with_secom_skip(self): 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 ---