Repository navigation
fix: wrong contacts on tasks, and orchestrators that never delegate on cloud - #100
Merged
Merged
Conversation
`extract_entities` tested `name_variant in combined`, so a short name
matched inside any longer word: "ana" hit *Mariana*, *Susana*,
*analysis* and *banana*. A message mentioning only Mariana resolved to
BOTH Ana and Mariana, and whichever node id landed first won the
mention — that is the "wrong contact attached to a task" report.
`_build_name_index` compounded it. Every name part >2 chars was
indexed with `setdefault`, so when two contacts share a first name the
one indexed first permanently owned that token and every later mention
of the other person mis-resolved to them.
Three changes:
- New `src/core/name_matching.py` holds the rule in one place.
Boundaries use `(?<!\w)`/`(?!\w)` rather than `\b` so names that
begin or end with punctuation (O'Brien, Ana-Paula) still anchor, and
matching is Unicode-aware so accented names get real boundaries.
Patterns are lru_cached — the same contacts are rematched on every
query.
- Matching is longest-variant-first, so a full name claims a mention
ahead of a bare first name that overlaps it.
- The index now accumulates `variant -> {owner ids}` and keeps only
single-owner variants. A token two people share resolves to nobody
instead of to whoever was indexed first: callers can degrade from a
missing contact, but cannot detect a confidently wrong one.
`question_patterns.extract_entities` had the same substring test and
now shares the helper.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Pro builds switch cloud models to native structured output (overlay
patch 08) because the gateway honours `response_format` but ignores
`tool_choice="required"`. That patch's comment claims regular auto tool
calls still work. For agents that delegate, they do not.
Measured against DeepInfra, same prompt, same tools, only the profile
field changed:
default_structured_output_mode="tool" -> ['goal_extractor']
default_structured_output_mode="native" -> [] (every time)
The orchestrator answers directly and never calls a `delegate_*` tool,
so on cloud `task_curator` never reaches `goal_extractor` /
`task_proposer` and no goals or tasks are ever produced. Consistent
with `task_proposer` being absent from `agent_run_log` entirely while
`goal_extractor` shows 133 runs.
Fix scopes the mode per agent instead of per endpoint: `SBOrchestrator`
and `SBDeepAgent` carry `delegates = True`, and `_output_spec_for()`
wraps their `output_type` in `pydantic_ai.ToolOutput`. Every other
agent keeps the model profile's default, so the native path that patch
08 was added for is untouched.
It lands at the single `Agent(...)` construction site in
`_get_pa_agent`, so `ModelFactory` and its endpoint cache — which is
what makes bearer rotation work — are not touched.
Verified with the native profile forced: the three routing probes that
returned `[]` now delegate to `goal_extractor`, `task_proposer` and
`daily_scheduler` respectively. Still worth one live call through
gateway.arandu.life before shipping, since tool-mode against the real
gateway is the premise patch 08 doubted.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The two agents behind the "goals and tasks extraction is wrong" reports had no eval coverage, so there was no way to tell whether a model swap helped. `task_proposer` (7 cases) covers the judgement calls that actually go wrong: task vs pending reply, and who owes what. Cases carry near-miss names (Ana/Mariana, Bruno/Brunela) so a model that pattern-matches on a name prefix fails. `task_curator` (9 cases) grades routing only. The five children are replaced with stubs, so a case costs one curator call instead of fanning out into a real goal-extractor or scheduler run. `DelegatedTo` requires an exact match, so over-delegating fails too — every extra hop is a paid call the user waits on. Two harness fixes found while building these: - `Dataset.evaluate_sync` runs cases concurrently, so the curator adapter installs its stubs once and collects delegations through a ContextVar. Swapping registry entries around each case raced: one case restored the real definitions while another was mid-run, and every row scored "delegated to nobody". - `ContainsIds` stringified list-valued keys, so a `source_message_ids` of `["m1"]` became `"['m1']"` and never matched the expected `"m1"` — four grounding assertions failed for every model regardless of output. List values are now flattened; scalar keys are unaffected. The adapters also register their agent family explicitly, since the registry is empty in a bare eval process. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Neither SBAgent had a production caller — both were instantiated only by the eval adapter, so every logged run was the eval suite and no user ever saw their output. - ``InsightAgent``: ``InsightGenerator`` (which does fill the Dashboard) authors prose through ``BrainAgentV2.ask()`` at three sites and never touched the agent. The generator, its ``_insights`` table and the Dashboard cards are untouched — only the orphaned wrapper is gone. - ``WeeklyDigestAgent``: its docstring claimed the legacy extension "delegates to" it; the extension actually calls ``context.ask_llm()`` directly. That extension is removed too — it has never run on this install (no ``ext_weekly_digest_summaries`` table) and is not installed as a connector. Removes both agents, the weekly-digest extension, their registrations and Brain delegation entry, the ``insight``/``weekly_digest`` eval suites and adapters, the ``InsightDraft``/``DigestSummary`` output types, the frozen ``weekly_digest_v1`` prompt and its golden fixture. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
A full pass is 213 cases across 29 suites, ~480 LLM calls once the injection-firewall companions are counted, and it bills the user's real cloud balance when the provider is arandu_cloud — about $0.16 a run, measured. Re-running an unchanged suite only reproduces a known answer, and the suites hit exactly the agents now on the premium tier. ``run_suite`` now hashes the three inputs that can change a result — the model serving the agent, its resolved system prompt, and the dataset bytes — and skips when that fingerprint has already passed. Design notes: - Only ``status='ok'`` suppresses. A failed run always re-runs, because confirming whether the failure persists is the point. - Matching is "has this fingerprint EVER passed", not "was it the most recent". An A -> B -> A edit then skips on the return to A, which is the case we can be surest about. - An unresolvable input yields ``None``, which means "cannot prove unchanged" and the suite runs. The gate can only ever save a run it is certain about. - ``--force`` overrides. The fingerprint does not cover agent *source*, so use it after editing agent code. Two supporting fixes: - CLI runs are now recorded in ``agent_eval_runs`` with ``trigger='cli'``. Only the Agents-page button used to write there, so a ``make evals`` left no trace and eval spend was unattributable — which is why this cost had to be reverse-engineered from the run log. - ``suite_fingerprint`` bootstraps the agent registry once per process. A bare ``python -m evals.run_evals`` imports nothing that registers agents, so every fingerprint came back ``None`` and the gate never fired. Adds a guarded ``fingerprint`` column; installs predating this migrate in place. The README's "~$0.01 per full run" claim is corrected — it was off by more than an order of magnitude. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
Replaces the `_REGISTRY_READY` module global CodeQL flagged. The `global` + read + rebind idiom is exactly the shape its unused-global-variable query mishandles, and `lru_cache(maxsize=1)` says "run once per process" more directly anyway. It also caches the failure path, which is the behaviour we want: a registry that could not be built will not build on the next call, and retrying per suite would re-pay the import cost to reach the same `None`. Co-Authored-By: Claude Opus 5 <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
2 participants
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.
Six commits, reviewable independently.
1. Contact names matched as bare substrings
extract_entitiestestedname_variant in combined, so"ana"matched inside Mariana, Susana, analysis and banana. Demonstrated against the old code:Both contacts attached to a message naming only one — the "wrong contact on a task" report.
_build_name_indexcompounded it: every name part >2 chars was indexed withsetdefault, so when two contacts share a first name the first-indexed one permanently owned that token.Now: whole-word matching anchored with
(?<!\w)/(?!\w)(soO'BrienandAna-Paulastill work, and accented names get real boundaries), longest-variant-first, and an index keeping only single-owner variants — a token two people share resolves to nobody rather than to whoever came first.2. Orchestrators never delegate under native structured output
Pro builds force native structured output for cloud models (overlay patch 08), whose comment claims auto tool calls still work. Measured against DeepInfra, same prompt and tools, only that field changed:
tool['goal_extractor']native[]— every timeSo on cloud
task_curatornever reachestask_proposer, and no tasks are produced.Fixed per agent rather than per endpoint:
SBOrchestrator/SBDeepAgentcarrydelegates = Trueand getToolOutput; every other agent keeps the profile default, so the native path patch 08 needs is untouched. No change toModelFactoryor its cache.Wants one live check through
gateway.arandu.lifebefore shipping — tool-mode working there is the premise patch 08 doubted. Verified against DeepInfra directly: all three probes now route correctly.3. Eval suites for
task_proposer/task_curatorNeither agent had coverage, so model changes couldn't be evaluated. Also fixes two harness bugs: cases run concurrently (the curator adapter now uses a ContextVar instead of racing on registry swaps), and
ContainsIdsstringified list-valued keys so["m1"]never matched"m1".These suites were then used to score seven candidate models for arandu-cloud#32.
4. Delete the unused
insightandweekly_digestagentsNeither SBAgent had a production caller — both were instantiated only by the eval adapter, so every logged run was the eval suite and no user ever saw the output.
InsightAgent:InsightGenerator(which does fill the Dashboard) authors prose viaBrainAgentV2.ask()at three sites and never touched the agent. The generator,_insightsand the Dashboard cards are untouched — only the orphaned wrapper is gone.WeeklyDigestAgent: its docstring claimed the legacy extension delegates to it; the extension callscontext.ask_llm()directly. That extension is removed too — it has never run (noext_weekly_digest_summariestable) and is not installed.5. Skip an eval suite when nothing that affects it changed
A full pass is 213 cases /
480 LLM calls, billed to the user's real balance when the provider is$0.16 a run**, measured over six batch days. Re-running an unchanged suite reproduces a known answer.arandu_cloud— **run_suitenow hashes the model, the resolved system prompt and the dataset bytes, and skips when that fingerprint has already passed.--forceoverrides. The fingerprint does not cover agent source — use--forceafter editing agent code.Two supporting fixes: CLI runs are now recorded in
agent_eval_runswithtrigger='cli'(only the Agents-page button used to write there, somake evalsleft no trace and eval spend was unattributable), andsuite_fingerprintbootstraps the registry once per process — a barepython -m evals.run_evalsregisters no agents, so every fingerprint came backNoneand the gate never fired. GuardedALTER TABLE ... ADD COLUMN fingerprintfor existing installs.The README's "~$0.01 per full run" is corrected.
6.
lru_cachelatch for the registry bootstrapAddresses the CodeQL unused-global-variable alert on
_REGISTRY_READY.lru_cache(maxsize=1)states "once per process" more directly and also caches the failure path, which is the behaviour we want.On the two remaining CodeQL alerts
Both flag
evals/run_evals.pyimportingsrc.agents.eval_runneras beginning an import cycle. The cycle is real but pre-existing —eval_runner.pyhas importedevals.run_evalsat lines 490 and 527 since before this PR. Both directions use function-level imports, which is precisely the standard mitigation: nothing cycles at import time.The suggested autofix (
importlib.import_module("src.agents.eval_runner")) does not remove the cycle — it is the same import, hidden from static analysis, at the cost of type checking and IDE navigation. I have not applied it. Untangling the eval_runner ↔ evals.run_evals dependency properly means relocatingAGENT_SUITE_MAP, which belongs in its own PR.Testing
2,967 passed. The 9 failures are pre-existing and fail identically on a clean tree (5 WhatsApp bridge, 2 sync lifecycle, 2 worker reindex).🤖 Generated with Claude Code