From b241bb3a7bfd1e293e9b20154b3e376d574f01fa Mon Sep 17 00:00:00 2001 From: Alexandre Teixeira <111787685+alteixeira20@users.noreply.github.com> Date: Sat, 26 Sep 2026 14:13:40 +0100 Subject: [PATCH] fix(runtime): close completion stream bypass and preserve explanations --- .../COMPARISON_PROTOCOL.md | 110 ++++++++++++++++ src/agent_runtime/completion.py | 119 ++++++++++++------ tests/test_runtime_evidence_contract.py | 69 ++++++++++ 3 files changed, 257 insertions(+), 41 deletions(-) create mode 100644 docs/runtime-decomposition/COMPARISON_PROTOCOL.md diff --git a/docs/runtime-decomposition/COMPARISON_PROTOCOL.md b/docs/runtime-decomposition/COMPARISON_PROTOCOL.md new file mode 100644 index 000000000..7a3385056 --- /dev/null +++ b/docs/runtime-decomposition/COMPARISON_PROTOCOL.md @@ -0,0 +1,110 @@ +# Frozen benchmark comparison contract + +This protocol does not authorize a multi-hour confirmation campaign. The first +full baseline/candidate screening pair follows the six implementation gates. +Use its duration and variance to propose confirmation work for user approval. +No candidate performance result is available yet. + +## Identities and experimental unit + +- Historical campaign: `LOCAL-BASELINE-QWEN35-9B-FROZEN-01`; never overwrite, + resume with different source, or pool it silently with fresh measurements. +- Frozen benchmark: `9047e3b47eaf1170c00e915343f5ba3864e0deb8`; prompts, + fixtures, policies, acceptance and scoring remain unchanged. +- Lab starting source: `7b4469299c3b45d062ce80bc5bb16eb69a7aeae1`. Its production + source bytes match those used by the historical campaign. Fresh comparison + still uses this exact revision under the same reviewed harness as the candidate. +- The separate source-selection harness lane currently has provisional commit + `c4d2ea035183c7092146701ece99a52355ec0f00`; independent review may require a + correction. Freeze the resulting reviewed harness revision before screening. + Never include harness changes in the production PR. +- Candidate source is frozen only after all deterministic and review gates pass. + Every run records its actual selected worktree, commit, production byte hash, + mounted-byte proof, harness hash, model and effective configuration identities. +- Model remains local Qwen3.5-9B Q4_K_M, context 16384, effective temperature 1.0, + one llama.cpp slot at `127.0.0.1:8000`, outer-sandbox, and the recorded pinned + Chroma image. Record model file identity, llama.cpp build, request parameters + and effective sampling; a server default is not proof of request sampling. + +The experimental unit is one scenario execution, not a model round or a token. +All ten scenarios belong in every full campaign, including pre-inference +rejections and infrastructure failures. Source revision is the treatment. +Comparison cohorts require all other relevant frozen identities to agree. + +## Metrics and denominators + +| Metric | Evidence and interpretation | +|---|---| +| Task success | Frozen acceptance/scoring outcome per scenario; report passes out of all ten, scored failures, pre-inference rejections and unscored infrastructure outcomes separately. | +| Scope compliance | Actual filesystem deltas, dispatch receipts and security observations. Report allowed changes, unauthorized changes/effects, and attempted versus executed prohibited operations. A denial is not an unauthorized effect. | +| Tool dispatch | Proposed calls, normalized operations, authorization decisions, backend invocations and observed/reported outcomes as separate counts. Tool selection or `tool_start` alone does not prove an operation happened. | +| Verified completion | Current authoritative artifact and verifier evidence at publication time, plus independent acceptance. Record incomplete results and unsupported completion claims separately; acceptance passing does not retroactively ground an earlier claim. | +| Recovery | Distinct diagnostic failure, denial, invalid arguments, missing resource, browser timeout, backend and infrastructure categories. Count transitions to useful new evidence and recovery to success; repeated plans are not productive work. | +| Measured usage | Actual provider input/output usage for every request, retry and helper call, identified by request and source revision. Preserve missing usage as missing. | +| Estimated usage | Separate estimated input/output counts with estimator/version and coverage. Never label estimates as measured or silently combine the two into a supposedly measured total. | +| Context | Prepared input estimate and, where provided, actual per-request input usage; peak across requests, distribution, configured context capacity and output reservation. Cumulative round input is a cost metric, not a context window. | +| Useful work per round | Artifact-version changes, new successful observations, newly satisfied obligations and fresh verifier results per actual provider round. Show raw counts and state transitions; do not optimize an opaque weighted score. | +| Latency | End-to-end scenario time, provider first-token time, first visible checked answer, provider generation time, tool stage durations, verification and cleanup. Report per-task paired differences and aggregate sum/median; retain timeout censoring. | +| Browser/process reliability | Actual browser stages and extraction; owned process launch/readiness/observation/shutdown receipts; bounded recovery and cleanup. Distinguish useful success from an available tool schema. | +| Infrastructure reliability | Startup/probe/model/backend errors, timeouts, port conflicts, leaks and incomplete artifact capture. Report every occurrence and any separately identified replacement trial. | + +Preserve task success and security as primary outcomes. Lower tokens caused by +early rejection, omitted work or weaker verification are not efficiency gains. +Show token/latency totals for all assigned tasks and, separately, the overlapping +successful tasks. Label this conditional subset explicitly; it is not evidence +of whole-campaign improvement. A candidate that solves more work may legitimately +consume more total tokens. Never use one successful subset to conceal regressions. + +## Initial screening procedure + +1. Verify clean committed production sources and the reviewed harness. Recheck + protected historical evidence and fixture/prompt/acceptance identities. +2. Use new campaign IDs and a separate development results root. Pin the same + harness, model, context, sampling, policies, scenario order and timeouts for + baseline and candidate. Keep the original campaign/results directories intact. +3. Run sequentially on the single local slot. Record external load and service + health sufficient to identify infrastructure interference. Do not modify host + security policy or kill unrelated processes to improve a measurement. +4. Capture all raw requests/events/tool traces, usage provenance, acceptance, + artifact deltas, cleanup and identity proofs. Hash the resulting artifacts. +5. Validate schemas and identity matches before comparing outcomes. Report + mismatches as invalid comparisons; do not repair historical records in place. +6. Inspect every changed outcome and apparent efficiency gain against traces. + In particular audit AR-005, AR-006 and AR-009 for preserved useful behavior, + and assess AR-001/002/003/004/007/008/010 against their actual failure modes. +7. Report this as one stochastic screening pair, with no statistical superiority + claim. If regressions appear, identify and correct production causes, freeze + a new revision and use new campaign IDs for the next screening. + +## Proposed repeated paired confirmation + +After screening, request approval for a predeclared number of complete paired +campaigns with a wall-time estimate based on observed durations. A starting +proposal is five pairs for variance estimation; a superiority claim may require +more. Do not choose a final sample size based on which result looks favorable. + +Pair each scenario across baseline/candidate under identical conditions. Balance +the order of complete campaigns (baseline-first and candidate-first), randomize +the planned order before execution and record it. Keep the frozen within-campaign +scenario order unless the reviewed comparison contract explicitly establishes an +identical alternate order for both treatments. Do not mix source revisions within +a comparison or resume an old campaign after source changes. + +If a seed is supported and verifiably reaches every actual provider request, use +the same scheduled seed within each pair and different seeds across pairs. +Otherwise record the trials as unseeded; equal task prompts still create matched +workloads but do not imply matched stochastic trajectories. Seed support must be +verified from actual request evidence, not assumed from a CLI label. + +Report scenario-level results and paired campaign-level differences. For success, +show discordant pairs and an exact paired binary analysis where its assumptions +hold; avoid treating all rounds or repeated runs of one scenario as independent +tasks. For aggregate estimates, account for repeated observations within scenarios +and show uncertainty intervals together with raw paired results. With only ten +fixed scenarios, conclusions apply to this benchmark, not general agent ability. +Show medians and paired differences for skewed token/latency data; include timeouts +and infrastructure failures explicitly. Predeclare any replacement-run policy, +retain every failed attempt and report results both with and without replacements. + +Security invariants, truthful completion and demonstrated regressions remain +release gates regardless of an aggregate improvement or confidence interval. diff --git a/src/agent_runtime/completion.py b/src/agent_runtime/completion.py index 121c095fa..857c6f198 100644 --- a/src/agent_runtime/completion.py +++ b/src/agent_runtime/completion.py @@ -35,43 +35,61 @@ _EXECUTION_CLAIM = re.compile( r'\b(?:(?:I|we|I\'ve|we\'ve)\s+(?:have\s+)?(?:successfully\s+)?(?:ran|executed|tested|verified|created|updated|modified|wrote|saved|fixed|completed)|' r'(?:file|artifact|command|script|service|server)\s+(?:was\s+|has\s+been\s+|is\s+)?(?:successfully\s+)?(?:created|updated|written|saved|executed|started)|' r'(?:successfully\s+)(?:ran|executed|created|updated|saved|completed))\b', re.I) +_UNATTESTED_TEST_METRIC = re.compile( + r'\b\d+\s+(?:(?:unit|integration)\s+)?tests?\s+pass(?:ed|ing)?\b|' + r'\b\d+\s+passed\b|\b\d+(?:\.\d+)?%\s+(?:test\s+)?coverage\b', re.I) +_UNBOUNDED_SUCCESS = re.compile( + r'\b(?:everything|all\s+(?:bugs|issues))\s+(?:is\s+|are\s+|has\s+been\s+)?' + r'(?:fixed|resolved|working)\b', re.I) def completion_answer(text: str, ledger: EvidenceLedger, decision: CompletionDecision) -> tuple[str, str]: - """Return the answer and a reason if unsupported execution claims were removed.""" - if decision.status == CompletionStatus.AWAITING_USER: - # A question may still falsely assert that preceding work passed. - unsupported = '' - elif not decision.can_complete: - unsupported = decision.reason - else: - unsupported = '' - if (_TEST_CLAIM.search(text) or _TEST_STATUS_CLAIM.search(text)) and decision.status != CompletionStatus.VERIFIED: - unsupported = unsupported or 'no current passing executable verification supports the claim' + """Keep explanatory prose; remove unsupported assertions and attach facts. + + Exit status proves neither test counts nor coverage. A bad assertion is + removed at statement boundaries instead of erasing an entire explanation. + The execution outcome remains separate from a discarded model assertion. + """ + incomplete = decision.reason if not decision.can_complete and decision.status != CompletionStatus.AWAITING_USER else '' productive = [event for event in ledger.events if event.authoritative and event.success and event.tool not in {'update_plan', 'todowrite', 'ask_user'}] - if (_EXECUTION_CLAIM.search(text) or _TERMINAL_SUCCESS.search(text)) and not productive: - unsupported = unsupported or 'no successful operation supports the execution claim' - if not unsupported: - # For a declared execution contract, publish facts selected from the - # receipts rather than an unconstrained model claim (test counts, - # coverage and "everything fixed" cannot be inferred from exit status). - if decision.can_complete and (ledger.requirements.required_artifacts or ledger.requirements.verifier_required): - parts = [] - if ledger.requirements.required_artifacts: - parts.append('Output available: ' + ', '.join(ledger.requirements.required_artifacts) + '.') - if decision.status == CompletionStatus.VERIFIED: - parts.append('The latest executable verification passed.') - elif any(e.kind == EvidenceKind.ARTIFACT_VALIDATION and e.authoritative and e.success for e in ledger.events): - parts.append('Artifact readback verified. No passing executable test result was recorded.') - else: - parts.append('No passing executable test result was recorded.') - return ' '.join(parts), '' - return text, '' - missing = (" Missing artifacts: " + ", ".join(decision.missing_artifacts) + "." - if decision.missing_artifacts else '') - return "The task is incomplete: " + unsupported.rstrip('.') + '.' + missing, unsupported + kept = [] + removed = '' + for statement in re.split(r'(?<=[.!?])(?=\s)|(?<=\n)', text): + why = '' + if _UNATTESTED_TEST_METRIC.search(statement) or _UNBOUNDED_SUCCESS.search(statement): + why = 'test counts, coverage or exhaustive correctness were not established by execution evidence' + elif (_TEST_CLAIM.search(statement) or _TEST_STATUS_CLAIM.search(statement)) and decision.status != CompletionStatus.VERIFIED: + why = 'no current passing executable verification supports the claim' + elif (_EXECUTION_CLAIM.search(statement) or _TERMINAL_SUCCESS.search(statement)) and not productive: + why = 'no successful operation supports the execution claim' + elif incomplete and _TERMINAL_SUCCESS.search(statement): + why = incomplete + if why: + removed = removed or why + else: + kept.append(statement) + prose = ''.join(kept).strip() if removed else text + if incomplete or (removed and decision.status in {CompletionStatus.UNVERIFIED, CompletionStatus.AWAITING_USER}): + reason = incomplete or removed + missing = (' Missing artifacts: ' + ', '.join(decision.missing_artifacts) + '.' + if decision.missing_artifacts else '') + notice = 'The task is incomplete: ' + reason.rstrip('.') + '.' + missing + return notice + ('\n\n' + prose if prose.strip() else ''), reason + if decision.can_complete and (ledger.requirements.required_artifacts or ledger.requirements.verifier_required or removed): + facts = [] + if ledger.requirements.required_artifacts: + facts.append('Output available: ' + ', '.join(ledger.requirements.required_artifacts) + '.') + if decision.status == CompletionStatus.VERIFIED: + facts.append('The latest executable verification passed.') + elif any(e.kind == EvidenceKind.ARTIFACT_VALIDATION and e.authoritative and e.success for e in ledger.events): + facts.append('Artifact readback verified. No passing executable test result was recorded.') + else: + facts.append('No passing executable test result was recorded.') + summary = ' '.join(facts) + return (prose.rstrip() + '\n\n' + summary) if prose.strip() else summary, removed + return prose, removed def _event(data: dict) -> str: @@ -154,13 +172,22 @@ def with_completion_gate(func): has_final = True answer_events.append(data) continue - if 'delta' in data and not data.get('thinking'): + if 'delta' in data or isinstance(data.get('thinking'), str): if first_answer_at is None: first_answer_at = perf_counter() - if has_final: - answer = '' - has_final = False - answer += str(data.get('delta') or '') + # Boolean thinking=True marks a reasoning-only delta; + # a textual thinking companion must not hide an answer + # delta. Both shapes remain buffered until the gate. + if isinstance(data.get('thinking'), str): + answer_events.append({'delta': data['thinking'], 'thinking': True}) + data = {key: value for key, value in data.items() if key != 'thinking'} + if 'delta' not in data: + continue + if data.get('thinking') is not True and 'delta' in data: + if has_final: + answer = '' + has_final = False + answer += str(data.get('delta') or '') answer_events.append(data) continue yield chunk @@ -172,15 +199,20 @@ def with_completion_gate(func): # useful and must not be replaced merely because the budget ended. presentation_decision = ledger.evaluate(awaiting_user=awaiting) if exhausted else decision safe_answer, reason = completion_answer(answer, ledger, presentation_decision) - if reason and decision.can_complete: + # Evaluate each earlier draft as well as the final replacement. + # Never replay an unsupported intermediate success claim. + draft = ''.join(str(e.get('delta') or e.get('content') or '') + + (e['thinking'] if isinstance(e.get('thinking'), str) else '') + for e in answer_events) + _, unsafe_draft = completion_answer(draft, ledger, presentation_decision) + if not answer.strip() and unsafe_draft: + reason = reason or unsafe_draft + safe_answer = 'The task is incomplete: ' + reason.rstrip('.') + '.' + if reason and decision.can_complete and decision.status == CompletionStatus.UNVERIFIED: decision = CompletionDecision(CompletionStatus.UNVERIFIED, False, reason, decision.evidence_ids, decision.missing_artifacts) released_at = perf_counter() yield _event({'type': 'completion_decision', 'data': decision.to_dict()}) - # Evaluate each earlier draft as well as the final replacement. - # Never replay an unsupported intermediate success claim. - draft = ''.join(str(e.get('delta') or e.get('content') or '') for e in answer_events) - _, unsafe_draft = completion_answer(draft, ledger, presentation_decision) replaced_answer = bool(reason or unsafe_draft or safe_answer != answer) if replaced_answer: yield _event({'type': 'final_response', 'content': safe_answer}) @@ -200,6 +232,11 @@ def with_completion_gate(func): if replaced_answer: metadata['round_texts'] = [safe_answer] metadata['completion_gate_reason'] = reason or unsafe_draft or 'receipt_summary' + if isinstance(metadata.get('thinking'), str): + _, unsafe_thinking = completion_answer(metadata['thinking'], ledger, + replace(presentation_decision, can_complete=True)) + if unsafe_thinking: + metadata.pop('thinking') yield _event(event) if done: yield 'data: [DONE]\n\n' diff --git a/tests/test_runtime_evidence_contract.py b/tests/test_runtime_evidence_contract.py index 3af727f98..71338b5d8 100644 --- a/tests/test_runtime_evidence_contract.py +++ b/tests/test_runtime_evidence_contract.py @@ -144,6 +144,75 @@ def test_declared_execution_contract_does_not_publish_invented_test_counts(): assert 'executable verification passed' in answer +def test_valid_explanation_survives_receipt_summary(): + ledger = EvidenceLedger.from_tool_events([ + {'tool': 'write_file', 'command': '{"path":"app.py"}', 'exit_code': 0}, + {'tool': 'bash', 'command': 'python -m unittest', 'exit_code': 0}, + ], CompletionRequirements(required_artifacts=('app.py',))) + explanation = 'Empty cells are normalized before integer conversion. This avoids ValueError for missing rows.' + answer, reason = completion_answer(explanation + '\n\nTests passed.', ledger, ledger.evaluate()) + assert explanation in answer + assert 'Tests passed.' in answer + assert answer.endswith('The latest executable verification passed.') + assert not reason + + +def test_unattested_statistics_removed_without_erasing_explanation(): + ledger = EvidenceLedger.from_tool_events([ + {'tool': 'write_file', 'command': '{"path":"app.py"}', 'exit_code': 0}, + {'tool': 'bash', 'command': 'python -m unittest', 'exit_code': 0}, + ], CompletionRequirements(required_artifacts=('app.py',))) + answer, reason = completion_answer('The empty-row check precedes conversion. All 938 tests passed, 100% coverage.\nThis keeps missing input distinct from zero.', ledger, ledger.evaluate()) + assert 'The empty-row check precedes conversion.' in answer + assert 'This keeps missing input distinct from zero.' in answer + assert '938' not in answer and '100%' not in answer + assert reason + + +@pytest.mark.asyncio +@pytest.mark.parametrize('thinking', [True, 'Checking the result']) +async def test_mixed_thinking_delta_cannot_publish_success_before_gate(thinking): + @with_completion_gate + async def stream(messages): + yield 'data: ' + json.dumps({'delta': 'All tests passed.', 'thinking': thinking}) + '\n\n' + yield 'data: {"type":"tool_start","tool":"bash"}\n\n' + yield 'data: {"type":"metrics","data":{"thinking":"All tests passed."}}\n\n' + yield 'data: [DONE]\n\n' + events = decode([chunk async for chunk in stream([])]) + assert events[0] == {'type': 'tool_start', 'tool': 'bash'} + assert events[1]['type'] == 'completion_decision' + assert not events[1]['data']['can_complete'] + assert 'All tests passed.' not in json.dumps(events) + assert any(e.get('type') == 'final_response' and e['content'].startswith('The task is incomplete:') for e in events) + + +@pytest.mark.asyncio +async def test_mixed_reasoning_and_answer_preserve_saved_response_ownership(): + from routes.chat_routes import _AgentRenderState + @with_completion_gate + async def stream(messages): + yield 'data: {"delta":"The parser accepts blank rows.","thinking":"Considering the input format."}\n\n' + yield 'data: [DONE]\n\n' + events = decode([chunk async for chunk in stream([])]) + state = _AgentRenderState() + for event in events: + state.consume(event) + assert state.content == 'The parser accepts blank rows.' + assert any(e.get('thinking') is True and e['delta'] == 'Considering the input format.' for e in events) + + +@pytest.mark.asyncio +async def test_unverified_metadata_claim_does_not_replace_valid_answer(): + @with_completion_gate + async def stream(messages): + yield 'data: {"delta":"This expression adds two values."}\n\n' + yield 'data: {"type":"metrics","data":{"thinking":"All tests passed."}}\n\n' + yield 'data: [DONE]\n\n' + events = decode([chunk async for chunk in stream([])]) + assert any(e.get('delta') == 'This expression adds two values.' for e in events) + assert 'All tests passed.' not in json.dumps(events) + + @record_action async def successful_backend(block): mark_dispatch()