From ca3399e1775da175cff214d8ef3b387319471918 Mon Sep 17 00:00:00 2001 From: Jeffrey Phillips Freeman Date: Sun, 5 Apr 2026 21:26:33 +0000 Subject: [PATCH] fix(acms): invoke SkeletonCompressor in ContextAssembler.assemble() to propagate skeleton context to child plans MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Added skeleton_fragments: tuple[ContextFragment, ...] field to ContextPayload in context_fragment.py - Enables carrying compressed skeleton fragments along with normal context. Extended ACMSPipeline.assemble() in acms_service.py - Introduced skeleton_ratio: float = 0.15 (default matching spec) and parent_fragments: tuple[ContextFragment, ...] | None = None parameters. - These same parameters are also added to ContextAssemblyPipeline.assemble() in acms_pipeline.py for consistency. Skeleton compression integration - In Phase 3 of both assemble() methods, computed skeleton_budget = int(budget.available_tokens * skeleton_ratio) and invoked self._skeleton_compressor.compress(parent_fragments, skeleton_budget). - Compressed skeleton fragments are included in the returned ContextPayload.skeleton_fragments, enabling propagation of skeleton context to child plans. Tests and behavior coverage - Added a TDD issue-capture Behave scenario (@tdd_issue @tdd_issue_3563) to demonstrate the fix. - Added four Behave unit test scenarios asserting: compressor invocation, correct arguments, skeleton presence in output, and skeleton_ratio budget enforcement. - Added a Robot Framework integration test: parent plan accumulates context → child plan spawned → child plan context contains non-empty skeleton. - Added skeleton-context-inheritance command to helper_acms_pipeline.py to support testing and manual verification. Key design decisions - skeleton_ratio defaults to 0.15 to align with the spec's --skeleton-ratio default. - parent_fragments is None by default to maintain backward compatibility (no skeleton compression when no parent context). - skeleton_budget is computed as skeleton_budget = int(budget.available_tokens * skeleton_ratio), deriving the skeleton budget from the total token budget. - Both ACMSPipeline and ContextAssemblyPipeline are fixed to maintain consistency across the codepath. ISSUES CLOSED: #3563 --- features/acms_pipeline.feature | 59 +++++++++ features/steps/acms_pipeline_steps.py | 115 ++++++++++++++++++ robot/acms_pipeline.robot | 9 ++ robot/helper_acms_pipeline.py | 97 ++++++++++++++- .../application/services/acms_pipeline.py | 29 +++++ .../application/services/acms_service.py | 29 +++++ .../domain/models/core/context_fragment.py | 9 ++ 7 files changed, 346 insertions(+), 1 deletion(-) diff --git a/features/acms_pipeline.feature b/features/acms_pipeline.feature index 55e039a95..7377a5e7c 100644 --- a/features/acms_pipeline.feature +++ b/features/acms_pipeline.feature @@ -613,4 +613,63 @@ Feature: ACMS v1 Context Assembly Pipeline Then the empty strategy selector should have been called And the payload strategies used should include "relevance" And the payload should contain 2 fragments + + # --------------------------------------------------------------------------- + # Issue #3563 — SkeletonCompressor invocation in assemble() (TDD capture) + # --------------------------------------------------------------------------- + + @acms_assemble @skeleton @tdd_issue @tdd_issue_3563 @issue_3563 + Scenario: TDD capture — assemble() invokes skeleton compressor when parent_fragments provided + Given the ACMS pipeline modules are available + And a spy skeleton compressor is registered + And the following context fragments: + | uko_node | content | score | tokens | + | project://app/a.py | alpha | 0.9 | 100 | + And a context budget with max_tokens 1000 and reserved_tokens 0 + And parent fragments for skeleton compression: + | uko_node | content | score | tokens | + | project://parent/x.py | parent code | 0.8 | 200 | + When I assemble with parent fragments and skeleton_ratio 0.15 + Then the spy skeleton compressor should have been called + And the spy compressor should have received the parent fragments + And the spy compressor skeleton_budget should be 150 + + @acms_assemble @skeleton @issue_3563 + Scenario: assemble() includes skeleton_fragments in the returned ContextPayload + Given the ACMS pipeline modules are available + And a spy skeleton compressor is registered + And the following context fragments: + | uko_node | content | score | tokens | + | project://app/a.py | alpha | 0.9 | 100 | + And a context budget with max_tokens 1000 and reserved_tokens 0 + And parent fragments for skeleton compression: + | uko_node | content | score | tokens | + | project://parent/x.py | parent code | 0.8 | 200 | + When I assemble with parent fragments and skeleton_ratio 0.15 + Then the payload skeleton_fragments should be non-empty + And the payload skeleton_fragments should equal the spy compressor output + + @acms_assemble @skeleton @issue_3563 + Scenario: assemble() skeleton_fragments is empty when no parent_fragments provided + Given the ACMS pipeline modules are available + And the following context fragments: + | uko_node | content | score | tokens | + | project://app/a.py | alpha | 0.9 | 100 | + And a context budget with max_tokens 1000 and reserved_tokens 0 + When I assemble with strategy "relevance" + Then the payload skeleton_fragments should be empty + + @acms_assemble @skeleton @issue_3563 + Scenario: skeleton_ratio governs the skeleton budget passed to the compressor + Given the ACMS pipeline modules are available + And a spy skeleton compressor is registered + And the following context fragments: + | uko_node | content | score | tokens | + | project://app/a.py | alpha | 0.9 | 100 | + And a context budget with max_tokens 2000 and reserved_tokens 0 + And parent fragments for skeleton compression: + | uko_node | content | score | tokens | + | project://parent/x.py | parent code | 0.8 | 200 | + When I assemble with parent fragments and skeleton_ratio 0.25 + Then the spy compressor skeleton_budget should be 500 And the first fragment content should be "alpha" diff --git a/features/steps/acms_pipeline_steps.py b/features/steps/acms_pipeline_steps.py index a7602df96..aa861f56c 100644 --- a/features/steps/acms_pipeline_steps.py +++ b/features/steps/acms_pipeline_steps.py @@ -1080,3 +1080,118 @@ def step_empty_selector_called(context: Context) -> None: assert context.empty_selector.called, ( "Expected _EmptySelector.select() to be called, but it was not" ) + + +# --------------------------------------------------------------------------- +# Issue #3563 — SkeletonCompressor invocation in assemble() steps +# --------------------------------------------------------------------------- + + +class _SpySkeletonCompressor: + """Spy skeleton compressor that records calls and returns fragments unchanged.""" + + def __init__(self) -> None: + self.called = False + self.received_fragments: tuple[ContextFragment, ...] = () + self.received_budget: int = 0 + self.output: tuple[ContextFragment, ...] = () + + def compress( + self, + fragments: tuple[ContextFragment, ...], + skeleton_budget: int, + ) -> tuple[ContextFragment, ...]: + self.called = True + self.received_fragments = fragments + self.received_budget = skeleton_budget + # Return the fragments unchanged (pass-through spy) + self.output = fragments + return self.output + + +@given("a spy skeleton compressor is registered") +def step_spy_compressor_registered(context: Context) -> None: + spy = _SpySkeletonCompressor() + context.spy_compressor = spy + context.pipeline = ACMSPipeline(skeleton_compressor=spy) + + +@given("parent fragments for skeleton compression:") +def step_parent_fragments(context: Context) -> None: + frags = [] + for row in context.table: + frags.append( + _make_fragment( + uko_node=row["uko_node"], + content=row["content"], + relevance_score=float(row["score"]), + token_count=int(row["tokens"]), + ) + ) + context.parent_fragments = tuple(frags) + + +@when("I assemble with parent fragments and skeleton_ratio {ratio:g}") +def step_assemble_with_parent_fragments(context: Context, ratio: float) -> None: + if not hasattr(context, "pipeline"): + context.pipeline = ACMSPipeline() + context.assemble_error = None + try: + context.payload = context.pipeline.assemble( + plan_id="01JQTESTPN00000000000000AB", + fragments=list(context.fragments), + budget=context.budget, + parent_fragments=context.parent_fragments, + skeleton_ratio=ratio, + ) + except (ValueError, ValidationError) as exc: + context.assemble_error = exc + + +@then("the spy skeleton compressor should have been called") +def step_spy_compressor_called(context: Context) -> None: + assert context.spy_compressor.called, ( + "Expected _SpySkeletonCompressor.compress() to be called, but it was not. " + "This confirms the bug: SkeletonCompressor is stored but never invoked in assemble()." + ) + + +@then("the spy compressor should have received the parent fragments") +def step_spy_compressor_received_fragments(context: Context) -> None: + assert context.spy_compressor.received_fragments == context.parent_fragments, ( + f"Expected compressor to receive parent_fragments, " + f"got {context.spy_compressor.received_fragments!r}" + ) + + +@then("the spy compressor skeleton_budget should be {expected_budget:d}") +def step_spy_compressor_budget(context: Context, expected_budget: int) -> None: + actual = context.spy_compressor.received_budget + assert actual == expected_budget, ( + f"Expected skeleton_budget={expected_budget}, got {actual}. " + f"skeleton_budget should be int(total_budget * skeleton_ratio)." + ) + + +@then("the payload skeleton_fragments should be non-empty") +def step_payload_skeleton_non_empty(context: Context) -> None: + assert context.payload.skeleton_fragments, ( + "Expected payload.skeleton_fragments to be non-empty, but it was empty. " + "The assembled context must include compressed skeleton fragments." + ) + + +@then("the payload skeleton_fragments should equal the spy compressor output") +def step_payload_skeleton_equals_spy_output(context: Context) -> None: + assert context.payload.skeleton_fragments == context.spy_compressor.output, ( + f"Expected payload.skeleton_fragments == spy output, " + f"got {context.payload.skeleton_fragments!r}" + ) + + +@then("the payload skeleton_fragments should be empty") +def step_payload_skeleton_empty(context: Context) -> None: + assert context.payload.skeleton_fragments == (), ( + f"Expected payload.skeleton_fragments to be empty (), " + f"got {context.payload.skeleton_fragments!r}" + ) diff --git a/robot/acms_pipeline.robot b/robot/acms_pipeline.robot index df24f7efc..74924f85a 100644 --- a/robot/acms_pipeline.robot +++ b/robot/acms_pipeline.robot @@ -63,3 +63,12 @@ Assemble With Size Budget Context View Log ${result.stderr} Should Be Equal As Integers ${result.rc} 0 Should Contain ${result.stdout} acms-assemble-size-budget-ok + +Skeleton Context Inheritance Parent To Child Plan + [Documentation] Verify parent plan context is compressed and propagated to child plan via SkeletonCompressor. + ... Issue #3563: SkeletonCompressor must be invoked in assemble() so child plans receive non-empty skeleton_fragments. + ${result}= Run Process ${PYTHON} ${HELPER} skeleton-context-inheritance cwd=${WORKSPACE} + Log ${result.stdout} + Log ${result.stderr} + Should Be Equal As Integers ${result.rc} 0 + Should Contain ${result.stdout} acms-skeleton-inheritance-ok diff --git a/robot/helper_acms_pipeline.py b/robot/helper_acms_pipeline.py index 9fe9d6735..d3c12dba9 100644 --- a/robot/helper_acms_pipeline.py +++ b/robot/helper_acms_pipeline.py @@ -334,6 +334,100 @@ def _cmd_assemble_size_budget() -> int: return 0 +def _cmd_skeleton_context_inheritance() -> int: + """Verify parent plan context is compressed and propagated to child plan. + + Simulates the context inheritance scenario: + 1. Parent plan accumulates context fragments. + 2. Child plan is spawned — assemble() is called with parent_fragments. + 3. The assembled ContextPayload.skeleton_fragments must be non-empty, + confirming the SkeletonCompressor was invoked and the skeleton is + available for the child plan. + + This is the integration test for issue #3563. + """ + from cleveragents.application.services.acms_service import DefaultSkeletonCompressor + + # --- Parent plan accumulates context --- + parent_fragments = ( + ContextFragment( + uko_node="project://parent/strategy.py", + content="class PlanStrategy: pass # parent plan strategy", + token_count=50, + relevance_score=0.9, + provenance=FragmentProvenance(resource_uri="project://parent/strategy.py"), + ), + ContextFragment( + uko_node="project://parent/executor.py", + content="class PlanExecutor: pass # parent plan executor", + token_count=40, + relevance_score=0.7, + provenance=FragmentProvenance(resource_uri="project://parent/executor.py"), + ), + ContextFragment( + uko_node="project://parent/context.py", + content="class ContextManager: pass # parent context manager", + token_count=35, + relevance_score=0.6, + provenance=FragmentProvenance(resource_uri="project://parent/context.py"), + ), + ) + + # --- Child plan is spawned: assemble with parent_fragments --- + child_fragments = [ + ContextFragment( + uko_node="project://child/task.py", + content="class ChildTask: pass # child plan task", + token_count=30, + relevance_score=0.8, + provenance=FragmentProvenance(resource_uri="project://child/task.py"), + ), + ] + budget = ContextBudget(max_tokens=1000, reserved_tokens=0) + skeleton_ratio = 0.15 + + pipeline = ACMSPipeline(skeleton_compressor=DefaultSkeletonCompressor()) + payload = pipeline.assemble( + plan_id="01JQTESTPN00000000000000AB", + fragments=child_fragments, + budget=budget, + parent_fragments=parent_fragments, + skeleton_ratio=skeleton_ratio, + ) + + # Verify skeleton_fragments is non-empty (compressor was invoked) + if not payload.skeleton_fragments: + print( + "acms-fail: payload.skeleton_fragments is empty — " + "SkeletonCompressor was not invoked in assemble(). " + "Child plan has no inherited parent context (issue #3563)." + ) + return 1 + + # Verify skeleton budget was computed correctly + expected_budget = int(budget.available_tokens * skeleton_ratio) + actual_tokens = sum(f.token_count for f in payload.skeleton_fragments) + if actual_tokens > expected_budget: + print( + f"acms-fail: skeleton tokens {actual_tokens} exceed budget {expected_budget}" + ) + return 1 + + # Verify child plan fragments are also present + if not payload.fragments: + print("acms-fail: child plan fragments are missing from payload") + return 1 + + print( + f"acms-skeleton-inheritance-ok: " + f"skeleton_fragments={len(payload.skeleton_fragments)} " + f"skeleton_tokens={actual_tokens} " + f"skeleton_budget={expected_budget} " + f"child_fragments={len(payload.fragments)}" + ) + return 0 + + _COMMANDS: dict[str, Callable[[], int]] = { "fragment-create": _cmd_fragment_create, "budget-calc": _cmd_budget_calc, @@ -342,6 +436,7 @@ _COMMANDS: dict[str, Callable[[], int]] = { "assemble-tiered": _cmd_assemble_tiered, "payload-budget-check": _cmd_payload_budget_check, "assemble-size-budget": _cmd_assemble_size_budget, + "skeleton-context-inheritance": _cmd_skeleton_context_inheritance, } @@ -352,7 +447,7 @@ def main() -> int: "Usage: helper_acms_pipeline.py " "" + "|assemble-size-budget|skeleton-context-inheritance>" ) return 1 diff --git a/src/cleveragents/application/services/acms_pipeline.py b/src/cleveragents/application/services/acms_pipeline.py index 6c3f774c7..95a9f4854 100644 --- a/src/cleveragents/application/services/acms_pipeline.py +++ b/src/cleveragents/application/services/acms_pipeline.py @@ -580,6 +580,8 @@ class ContextAssemblyPipeline(ACMSPipeline): strategy: str | None = None, request: ContextRequest | None = None, context_view: ContextView | None = None, + skeleton_ratio: float = 0.15, + parent_fragments: tuple[ContextFragment, ...] | None = None, ) -> ContextPayload: """Assemble context with per-stage timing instrumentation. @@ -587,6 +589,13 @@ class ContextAssemblyPipeline(ACMSPipeline): with per-stage millisecond timings. When *context_view* is provided, byte-size limits are enforced as a pre-filter before strategy orchestration. + + When *parent_fragments* are provided, the ``SkeletonCompressor`` + is invoked during Phase 3 to compress them into a skeleton for + child plan context inheritance. The skeleton budget is computed + as ``int(budget.available_tokens * skeleton_ratio)``. The + compressed fragments are included in the returned + ``ContextPayload.skeleton_fragments``. """ import re @@ -668,6 +677,25 @@ class ContextAssemblyPipeline(ACMSPipeline): preamble = self._preamble_generator.generate(fused) preamble_ms = (time.monotonic() - t0) * 1000 + # Phase 3: Skeleton compression for child plan context inheritance. + # Compute skeleton_budget from skeleton_ratio and invoke the compressor + # when parent_fragments are provided. + skeleton_budget = int(budget.available_tokens * skeleton_ratio) + skeleton_frags: tuple[ContextFragment, ...] = () + if parent_fragments: + skeleton_frags = self._skeleton_compressor.compress( + parent_fragments, + skeleton_budget, + ) + self._pipeline_logger.info( + "Skeleton compressed", + plan_id=plan_id, + skeleton_budget=skeleton_budget, + skeleton_ratio=skeleton_ratio, + input_fragments=len(parent_fragments), + output_fragments=len(skeleton_frags), + ) + total_ms = (time.monotonic() - pipeline_start) * 1000 self._last_timings = StageTimings( @@ -734,4 +762,5 @@ class ContextAssemblyPipeline(ACMSPipeline): context_hash=context_hash, preamble=preamble, provenance_map=provenance_map, + skeleton_fragments=skeleton_frags, ) diff --git a/src/cleveragents/application/services/acms_service.py b/src/cleveragents/application/services/acms_service.py index 78ad0d5f0..9e00ba7f4 100644 --- a/src/cleveragents/application/services/acms_service.py +++ b/src/cleveragents/application/services/acms_service.py @@ -776,6 +776,8 @@ class ACMSPipeline: strategy: str | None = None, request: ContextRequest | None = None, context_view: ContextView | None = None, + skeleton_ratio: float = 0.15, + parent_fragments: tuple[ContextFragment, ...] | None = None, ) -> ContextPayload: """Assemble context fragments into a budget-constrained payload. @@ -784,6 +786,13 @@ class ACMSPipeline: strategy orchestration. Violations are logged and the ``BudgetEnforcementResult`` is stored on the pipeline instance as ``last_enforcement_result`` for caller inspection. + + When *parent_fragments* are provided, the ``SkeletonCompressor`` + is invoked to compress them into a skeleton for child plan + inheritance. The skeleton budget is computed as + ``int(budget.available_tokens * skeleton_ratio)``. The + compressed fragments are included in the returned + ``ContextPayload.skeleton_fragments``. """ if not re.match(ULID_PATTERN, plan_id): msg = f"plan_id must be a valid ULID, got {plan_id!r}" @@ -844,6 +853,25 @@ class ACMSPipeline: # Phase 3: Context Finalization preamble = self._preamble_generator.generate(fused) + # Phase 3: Skeleton compression for child plan context inheritance. + # Compute skeleton_budget from skeleton_ratio and invoke the compressor + # when parent_fragments are provided. + skeleton_budget = int(budget.available_tokens * skeleton_ratio) + skeleton_frags: tuple[ContextFragment, ...] = () + if parent_fragments: + skeleton_frags = self._skeleton_compressor.compress( + parent_fragments, + skeleton_budget, + ) + self._logger.info( + "Skeleton compressed", + plan_id=plan_id, + skeleton_budget=skeleton_budget, + skeleton_ratio=skeleton_ratio, + input_fragments=len(parent_fragments), + output_fragments=len(skeleton_frags), + ) + final_fragments = tuple(fused) total_tokens = sum(f.token_count for f in final_fragments) available = budget.available_tokens @@ -870,6 +898,7 @@ class ACMSPipeline: context_hash=context_hash, preamble=preamble, provenance_map=provenance_map, + skeleton_fragments=skeleton_frags, ) @property diff --git a/src/cleveragents/domain/models/core/context_fragment.py b/src/cleveragents/domain/models/core/context_fragment.py index b3987d1f1..aa72fbf0a 100644 --- a/src/cleveragents/domain/models/core/context_fragment.py +++ b/src/cleveragents/domain/models/core/context_fragment.py @@ -209,6 +209,15 @@ class ContextPayload(CRPAssembledContext, frozen=True): description="Fragment ID -> provenance mapping for traceability", ) + skeleton_fragments: tuple[ContextFragment, ...] = Field( + default=(), + description=( + "Compressed parent-context fragments for propagation to child plans. " + "Produced by SkeletonCompressor during Phase 3 finalization. " + "Empty when no parent context is available or skeleton_ratio is 0." + ), + ) + @field_validator("provenance_map") @classmethod def _freeze_provenance_map(cls, v: dict[str, Any]) -> dict[str, Any]: -- 2.52.0