fix(wp4): close phase3 subsystem follow-ups#10
Merged
Conversation
The OpenRouter-backed `summary()` and inferred-edge dispatch paths landed in B.6 but B.8's red gate run surfaced three defects in the live path: - Live responses weren't using strict JSON response schemas, so semantic parsing of summary/inferred output was brittle against any provider whitespace or field-order drift. - `max_tokens` wasn't being passed on live calls, so OpenRouter defaulted to a soft cap that truncated some inferred-dispatch JSON. - Malformed-JSON parse failures lost token/cost stats, masking real spend. Fixes: - `llm_provider.rs`: switch to OpenRouter structured-outputs feature per https://openrouter.ai/docs/features/structured-outputs; thread `max_tokens` through live requests; preserve token+cost counters on semantic parse failure so the runs.stats record is honest even when the response can't be deserialized. - `mcp/lib.rs`: matching changes in the dispatch path; keep inferred output untruncated. - Tests: targeted Rust tests in storage_tools.rs covering OpenRouter malformed-JSON accounting + inferred dispatch + serve wiring + prompt templates. - Harness: driver.py + test_driver.py updates so the B.8 harness exercises the fixed path. Rerun evidence: - tests/perf/b8_scale_test/results/2026-05-17T2243Z/mcp-driver-output.json (cold run, 100/100 OK, summary_miss_then_hit=true, 57 inferred edges) - tests/perf/b8_scale_test/results/2026-05-17T2243Z/mcp-driver-output-warm-cache.json (warm run, 100/100 OK, p95 200.273ms, zero new tokens/cost) `docs/implementation/sprint-2/b8-results.md` is appended with the GREEN rerun entry superseding (but not erasing) the prior RED gate record; the `v0.1-sprint-2` tag stays at the close commit per the honest "RED partial milestone with GREEN rerun superseding" framing in signoffs.md. Verification: cargo build --release -p clarion-cli; targeted Rust tests for OpenRouter, malformed-JSON accounting, inferred dispatch, serve wiring, prompt templates; pytest -q tests/perf/b8_scale_test/ test_driver.py; git diff --check. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Prevents accidental commit of OpenRouter (or future provider) API keys used during B.8 scale-test runs and any subsequent live-LLM work. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
PEP 484 overload stubs share their qualname with the implementation, so emitting one entity per FunctionDef caused identical entity IDs to collide on the host writer-actor's UNIQUE constraint, failing analyze mid-run on any real src/ code that uses @overload (clarion-e29402d1ba — surfaced at elspeth scale on commit 9d3fd55d63). Recognise @overload / @typing.overload / @typing_extensions.overload in the extractor and skip stub emission entirely so the implementation alone becomes the entity. Suppress reference-site collection inside stubs too (their bodies are `...` and signature annotations are type-checker hints, not runtime references) — this is a small behavioural narrowing for any consumer that previously saw reference edges from stub annotations. Belt-and-suspenders: deduplicate any other same-id collisions (aliased overload imports, singledispatch.register `def _():` sequences, manual redefinition) first-wins with stderr + a new ExtractionStats.duplicate_entities_dropped_total counter so the host's UNIQUE never trips from plugin-emitted duplicates. Verified end-to-end on the elspeth landscape directory (the original reproducer's two @overload files): analyze completes with both ExecutionRepository.complete_node_state and serialize_datetime persisted as exactly one function entity each. Note: the existing pre-existing clarion-mcp clippy violations and the failing summary_token_ceiling_reserves_concurrent_cold_misses nextest case are on this branch already (verified via stash) and are unrelated to this change. A separate cross-file module-entity collision surfaced when re-running on the full elspeth corpus and is filed as clarion-0cd961dbbc (different mechanism, different layer). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
`clarion serve` and `clarion analyze` now pick up `OPENROUTER_API_KEY`, `FILIGREE_API_TOKEN`, `RUST_LOG`, and any other config-via-env values from a `.env` file walked up from the current directory, without requiring the parent shell to source it. Existing process env vars win over `.env` values (dotenvy default), so an explicit `OPENROUTER_API_KEY=… clarion …` still beats a checked-in dev `.env`. Missing `.env` is silently ignored. Motivated by the `.mcp.json` `clarion serve` entry: Claude Code's MCP loader has no shell stage to source `.env`, so without this the live-LLM paths (`summary`, inferred `callers_of`) would fail with "no API key" unless the user explicitly exported the var in whichever shell they launched Claude Code from. Two integration tests prove both directions: `.env`-supplied RUST_LOG=debug surfaces a DEBUG-level log line that the default `info` filter would suppress; an explicit `RUST_LOG=info` in the process env beats the `.env`'s `RUST_LOG=debug`. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…lspeth Two paired B.8 entries against the full elspeth checkout (/home/john/elspeth, 1,532 .py / 611k LOC), both unblocked by the @overload fix shipped in 29f0426. 2026-05-18T01:14Z — GREEN, storage-backed slice. analyze completed end-to-end in 484.65s / 8m04s (≤ 13.46% of NFR-PERF-01); 33,250 entities / 87,263 persisted edges; storage-backed MCP surface stayed sub-15ms p95 across all 7 tools with the inferred-edge LLM dispatch deliberately skipped to keep the named-slice GREEN as the only inferred-path verdict. 2026-05-18T02:06Z — GREEN amendment, live-LLM path. Same analyze DB, caches wiped, driver re-run without --skip-inferred to exercise the inferred-edge dispatch and cold-cache summary path. 99/100 OK, $0.897 cost on 219k tokens, summary-cache stabilises to 1.0 hit rate in steady-state. One named follow-up filed: clarion-df58379de4 (inferred- edge persistence rejects LLM-proposed entity IDs with FK violation and retryable=true causes cost amplification). surfaced-and-dismissed during this gate: clarion-0cd961dbbc — filed mid-investigation as a suspected cross-file UNIQUE collision, then closed as not_a_bug after diagnosis showed the duplicate was stale state in the corpus's .clarion DB from the prior 2026-05-18T00:17Z RED run, not a real plugin defect. Includes raw artefacts under tests/perf/b8_scale_test/results/ 2026-05-18T0114Z/ (analyze metrics + stdout/stderr, RSS sampler, storage-backed and live-LLM driver outputs). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
CHECK constraints on closed core-owned vocabularies (findings.{kind,
severity,status}, runs.status) per ADR-031; entities.kind / edges.kind
remain unconstrained because ADR-022 reserves those to the plugin
manifest. Writer-actor and manifest acceptance stay the canonical
enforcement; CHECKs are defense-in-depth against hand-typed literals,
silent enum drift, and out-of-band debug writes. Closes F-13 from the
2026-05-03 skeleton audit and Filigree clarion-fbe50aa6e1.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
ADR-022 requires at least one `-[A-Z0-9]+` segment after `CLA`, so the quantifier in the docstring is `+`, not `*`. The runtime check already enforces this via the `segments.len() < 2` guard; only the docstring description was misaligned. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
clarion-ac5f9bf35b is closed; the OpenRouter-backed cold/warm rerun documented in b8-results.md supersedes the original RED verdict for live summary(), inferred dispatch, and cost/cache proof. v0.1-sprint-2 tag remains pinned to the historical RED close commit; GREEN evidence sits after the tag. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Raw analyze/db metrics and corpus snapshots referenced by b8-results.md:426 for the 2026-05-18T0017Z RED slice on the full elspeth corpus. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Auto-managed filigree instruction block; documents the seven cross-product entity-association MCP/HTTP surfaces added in ADR-029. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
clippy needless_pass_by_value flagged stats_delta: Value and diagnostics: Vec<Value> in tool_error_envelope_with_diagnostics because the json! macro hides the move-into-Map from clippy's flow analysis. Constructing the serde_json::Map manually makes consumption explicit without cloning or adding an #[allow] override. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
BudgetReservation::commit treated budget.blocked as a kill-switch for the in-flight call, but blocked is a "no new reservations" gate. A reservation that already cleared reserve_budget paid for its dispatch slot at approval time; a concurrent caller that overflows the ceiling and sets blocked must not retroactively void it. Concretely: with two cold summaries racing under a 150-token ceiling and 120-token estimates, the first reserves 120 OK, the second fails reserve_budget and sets blocked=true, and the first then drops its reservation before checking budget.spent + actual against the ceiling. With the old check the first also commit-failed with token-ceiling-exceeded; both calls returned ceiling envelopes, no provider work landed, and the cold-miss reservation guarantee was lost. Remove `budget.blocked ||` from the commit guard; the actual-overrun check (`spent + actual > ceiling`) is preserved, so genuine overruns still fail and still set blocked. Fixes the long-failing summary_token_ceiling_reserves_concurrent_cold_misses regression test. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
clarion-df58379de4: B.8 live-LLM rerun surfaced two compounding defects on the inferred-edge dispatch path. 1. The LLM can return well-formed JSON whose `target_id` doesn't exist in the `entities` table. The writer-actor's FK-protected INSERT then fails with `FOREIGN KEY constraint failed`, which aborts the whole transaction — including the `inferred_edge_cache` upsert. The cache row never lands, so a retry re-burns LLM tokens. 2. The `storage-error` envelope marked every storage failure `retryable=true`, but FK violations are deterministic against the same row set. A client honouring the hint loops and pays again. The fix: - New `existing_entity_ids` reader query (`clarion-storage::query`) returns the subset of candidate IDs that resolve in `entities`, chunked under the default SQLite parameter cap. - `ServerState::drop_unresolved_inferred_targets` calls it via the reader pool after `inferred_records_from_result`, drops records whose `to_id` is hallucinated, and folds the count into a new `inferred_unresolved_targets_dropped_total` field on `InferredDispatchStats` (both cache-miss and cache-hit paths). - `StorageError::is_foreign_key_violation` matches the SQLite extended code 787; `InferredDispatchFailure::from_storage` and a new `storage_retryable` helper used by every `storage-error` envelope classify FK violations as `retryable=false`. Tests: - `callers_of_inferred_drops_targets_missing_from_entities_table`: LLM proposes a phantom `target_id`; the dispatch returns `ok=true` with `inferred_unresolved_targets_dropped_total=1` and 0 materialised edges; the cache row lands so the warm rerun does not re-dispatch. - `is_foreign_key_violation_*` unit tests in `clarion-storage::error` prove the classifier matches FK breaches, rejects PK collisions, and returns false for non-SQLite variants. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Adds a `clarion` entry to `.mcp.json` pointing at the locally-built `target/release/clarion serve` binary against the B.8 elspeth corpus checkout. Paths are absolute and developer-local (`/home/john/clarion/...`, `/tmp/clarion-b8-elspeth-full-20260518T0016Z/...`); other contributors will need to rewrite them before the entry resolves on their machine. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Five-stage architecture analysis output (coordination, discovery findings, subsystem catalog, diagrams, final report) plus the per-section and validation scratch under temp/. Snapshot of the clarion-* workspace as it stood at sprint-2/b8-scale-test. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
tachyon-beep
marked this pull request as ready for review
May 18, 2026 15:30
8 tasks
tachyon-beep
added a commit
that referenced
this pull request
May 30, 2026
Three defects in the new agent-orientation install feature, found in the codex review of PR #21. All latent at 1.0 (the feature has never shipped), fixed before merge per request. - #5 shell-escape the embedded hook path: the SessionStart command embedded the project path in double quotes, which does not neutralize `$`, backtick, or `\`. Add POSIX single-quote escaping (shell_single_quote) so the path is always a single literal argument. Test round-trips metacharacters through a real `sh`. - #10 refresh a stale Clarion hook: the idempotency check keyed on the HOOK_COMMAND substring and no-oped whenever any matching hook existed, so a pre-existing path-less hook (or one pinned to another project) was never updated on re-install. Now: exact-match -> no-op; Clarion-owned but different -> refresh in place; absent -> append. Idempotency preserved. - #6 reject a non-directory .clarion under --all: when .clarion existed as a regular file and a non-bare init ran without --force, install skipped DB creation and "succeeded" with skills/hooks atop a project with no clarion.db. Now bails with a clear non-directory error. Test asserts the failure (previously exited 0). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Verification