Authorship grounding: fix issue #29 across emit, memory, tools, and prompts - #32
Draft
ahueb wants to merge 203 commits into
Draft
Authorship grounding: fix issue #29 across emit, memory, tools, and prompts#32ahueb wants to merge 203 commits into
ahueb wants to merge 203 commits into
Conversation
Cohorts are admin-managed groups gating which agents act on each other's activity during simulation, to conserve LLM calls at large roster sizes. Isolation (opt-in via cohort_isolation_enabled, default off): - New cohorts / cohort_memberships tables (migration 0019) + models. - SimulationEngine recomputes each agent's allowed_sender_ids on the roster sync cadence; the gate is applied at the MessageLog read boundary (get_new_top_level_posts / get_tags_for_agent / get_replies_to_agent_posts) so Phase 2 scan and Phase 3 activation skip non-cohort senders and Phase 4/5 Opus calls are suppressed downstream. Human PI messages always pass. Uncohorted agents are isolated when the flag is on. A Phase 5 cold-tag guard strips @tags toward non-cohort agents. - Admin CRUD at /admin/cohorts (list/create/detail/delete/add/remove) + nav. Reactive-priority scheduler (sequential; replaces the abandoned concurrent-turn proposal): _select_agent now selects owed-reply agents first (oldest-waiting, excluding the last LLM caller) so 1:1 threads conclude promptly instead of waiting on staleness-weighted random selection, with a fairness valve (max_consecutive_reactive_turns) to avoid starving new-conversation formation. The weighted-random proactive path and the back-to-back guard are unchanged. Tests: tests/test_cohort_isolation.py covers the MessageLog filter (incl. backward-compat when disabled), allowed_sender_ids recomputation, _owes_reply, and the reactive tier. Full suite green (282 passed). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…+ rebuild) Historically Slack was the primary durable store of conversation content: the in-memory MessageLog was rebuilt from Slack history on every start, and agent_messages held only metadata (message_length, no body). With Slack off the system could not reconstruct any conversation and a restart lost all history. This lands the foundation from specs/local-db-conversations.md so the local DB is the single source of truth and the simulation can run with Slack fully off. Stage 1 — content persistence + DB rebuild: - Migration 0019 extends agent_messages with content, sender_name, is_bot, posted_at and nullable slack_ts/slack_channel_id/slack_thread_ts mirror columns; relaxes agent_id to nullable (NULL = human/PI, mirroring LogEntry.sender_agent_id); adds UNIQUE(simulation_run_id, message_ts) plus posted_at / channel / partial slack_ts indexes. - MessageLog.append is now idempotent (skips duplicate ts) and fires a persist callback; load_entry() appends restored rows without re-persisting. - SimulationEngine buffers every appended entry (bot, peer, and human) and batch-upserts them into agent_messages in _flush_persisted (ON CONFLICT on run+message_ts), drained each main-loop tick and on stop(). The per-post _log_message path is retired. - _rebuild_state_from_db() hydrates the log from agent_messages; the per-agent state reconstruction is extracted to _rebuild_agent_state() so it runs with Slack on or off; _rebuild_state_from_slack is demoted to a Slack-only reconcile that only adds messages missing from the DB. Stage 2 — local id minting: - mint_ts() yields monotonic, unique, ts-shaped ids seeded from the rebuild's max(posted_at); replaces the str(time.time()) fallbacks and removes the "mock_ts" constant (a latent collision that idempotent append would have turned into message loss). - Slack-off channels use stable local:<name> ids (can't collide with Slack C…/G…); seeded channels are persisted to agent_channels. Verification: full suite 308 passed (new tests cover idempotent append, the persist callback, load_entry bypass, and mint_ts monotonicity/seeding), plus a real-Postgres continuity check (post -> persist -> fresh rebuild with thread intact, duplicate dropped, human row stored with NULL agent_id). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Formalizes running with Slack fully off and adds the Slack-independent inbound path for PI messages. - New src/agent/transport.py: a Transport Protocol matching the exact surface the engine uses on AgentSlackClient (which conforms structurally, no change), plus NullTransport — a no-op used when Slack is disabled. NullTransport reports is_connected=False (so the engine's existing guards take the no-op path), returns None from outbound posts (engine mints a local id), and returns local: ids for channel creates and [] from all inbound polls. - config.slack_enabled (bool | None): None auto-detects (Slack on iff any agent has a usable token); SLACK_ENABLED=false forces DB-only mode. main.py resolves it (--mock forces off), builds AgentSlackClients when on and NullTransports when off, and passes the flag to the engine. - SimulationEngine.slack_enabled gates the roster hot-add: when off, newly-active agents are admitted with a NullTransport instead of being dropped for a missing token / failed connect. - New _poll_pi_inbox_from_db(): ingests human/PI rows the web app writes to agent_messages (is_bot=false), appends any unseen ones to the MessageLog, and routes them through PI handling (proposal-review clear, thread reopen, pi_context, @bot tags) derived from the thread's own participants rather than a Slack user→agent map. Runs every tick regardless of Slack, and is the convergence point for the future Slack mirror's inbound side. Cursor seeded past restored history in _rebuild_state_from_db. Verification: full suite 314 passed (new tests: NullTransport behavior + Protocol conformance for both NullTransport and AgentSlackClient), plus a real-Postgres smoke test (a web-written PI reply on a proposal thread is ingested and clears the pending-proposal block; re-poll is a no-op). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
… poller The public→collab_private migration (PI reopen flow) now works with Slack off, and a running sim ingests migration handover posts through the DB. - private_channels.py: _slack_enabled_for_migration() resolves the mode (explicit SLACK_ENABLED wins; else auto-detect from whether both bots have usable tokens). When off, _migrate_offline() creates the collab_private AgentChannel with a local: id and the same PrivateChannelMember rows as the Slack path, but makes no Slack calls: the handover posts (authored by the creator bot) and the neutral ⏸️ origin-thread close marker are written straight to agent_messages (visibility=collab_private for the handover) and refined_in_channel is set. The other PI is not DM'd (no transport). - Generalized the engine's DB inbound poller (renamed _poll_pi_inbox_from_db -> _poll_inbound_from_db): it now ingests ANY unseen message for the run, not just human rows — so bot-authored handover posts written by the web process reach the live MessageLog. Human/PI rows still additionally go through PI handling; the sim's own messages are already in the log and are skipped. Verification: full suite 314 passed, plus a real-Postgres smoke test with SLACK_ENABLED=false (offline migration creates a local: private channel with 3 members, writes 3 handover posts + origin close marker + refined_in_channel, and a running sim ingests all 3 handover posts from the DB). Also confirmed the auto-detect path still uses Slack when tokens are present. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Gives PIs (and delegates) a Slack-independent way to see recent activity and
inject a message into their agent's workspace.
- src/services/pi_inbox.py: get_latest_run_id() and record_pi_message(), which
insert a human/PI row (is_bot=false, agent_id=null) into agent_messages with a
minted ts, resolving channel_id/visibility from agent_channels. The running
sim ingests it via _poll_inbound_from_db and routes it through PI handling.
- agent_page.py: GET /{agent_id}/conversations (a read view of recent messages —
content now lives in the DB — plus a post form) and POST /{agent_id}/message
(writes the inbound row; an optional "address my bot" toggle prepends the
@botName tag the engine already understands). Both gated by
get_agent_with_access (owner or delegate).
- templates/agent/conversations.html + a dashboard card linking to it.
Identity uses AgentRegistry.user_id / access checks rather than a Slack user id,
so this works with Slack fully off. DM-style directives depend on DM persistence
and are wired in Stage 7.
Verification: full suite 314 passed; a real-Postgres smoke test (record_pi_message
-> _poll_inbound_from_db ingests the PI row and the @bot tag sets has_pi_directive
on the target agent); app restarted cleanly and the new routes resolve.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Makes the non-engine Slack writers degrade to the DB when Slack is off, and records the Slack↔DB id mapping so mirroring and reconcile dedup are correct. Secondary posters (gated by slack_tokens.slack_globally_enabled — explicit SLACK_ENABLED wins, else auto-detect from token presence): - grantbot.py: when Slack is off, funding opportunities are written to agent_messages (authored by a "grantbot" identity, 💰 top-level post) via _post_funding_to_db instead of released — so funding threads still exist and agents scan them. - email_inbound.py + agent_page.py reopen: the legacy "post guidance to the origin thread" fallback now writes to the DB inbox via record_pi_message when Slack is off (the primary reopen path already routes through the Slack-aware migration). invite.py's users_lookupByEmail was already token-guarded. Slack-mirror mapping: - LogEntry gains slack_ts/slack_channel_id; _post_message records them when a connected client posted (in pure Slack-on mode slack_ts == message_ts), and _flush_persisted upserts slack_ts/slack_channel_id/slack_thread_ts. - _rebuild_state_from_db seeds _known_slack_ts; the Slack reconcile skips any message already represented in the DB by its slack_ts (dedup for a DB-origin message later mirrored to Slack) and stamps reconciled entries with their slack mapping. Verification: full suite 314 passed, plus a real-Postgres smoke test (a connected post persists message_ts == slack_ts + slack_channel_id, and a fresh rebuild seeds _known_slack_ts for reconcile dedup). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…ackfill
Completes the DB-primary refactor: PI<->bot DMs are durable and Slack-optional,
and there's a one-time importer for existing Slack history.
DM persistence:
- Migration 0020 + PiDmMessage model: pi_dm_messages (run, agent_id, pi_user_id,
direction inbound/outbound, content, ts, slack_ts, posted_at). pi_user_id is a
Slack user id (Slack-on) or local:<users.id> (web).
- pi_inbox.record_pi_dm() / web_pi_user_id(); PIHandler._send_dm now records every
outbound DM (durable + visible in the web UI even with Slack off) and takes
simulation_run_id.
- DM processing is unified through the DB: _poll_pi_dms (Slack) now only RECORDS
inbound DMs as rows; the new _poll_pi_dms_from_db is the single processor that
runs each inbound row through PIHandler.handle_dm and flips has_pi_directive.
This handles Slack and web DMs identically with no double-processing.
_seed_pi_dm_cursor starts it past existing history on boot.
PI web interface (DMs): POST /agent/{id}/dm writes an inbound DM row; the
conversations page shows the recent DM thread and a send box.
Ops:
- --fresh now also wipes pi_dm_messages.
- scripts/backfill_slack_history_to_db.py: one-time importer that reuses the
engine's setup + Slack reconcile + flush to pull channel/thread history
(content + slack_ts) into agent_messages for a run, without running any turns.
Verification: full suite 314 passed; a real-Postgres smoke test (inbound web DM
recorded → processed exactly once → directive flag set → re-poll no-op; outbound
DM persisted); app/worker/grantbot restarted cleanly; backfill script imports.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…els)
_persist_seeded_channels referenced AgentChannel but it wasn't in the module
import, raising NameError ("Failed to persist seeded channels") on startup so
seeded channels were never recorded in agent_channels. Surfaced by the first
Slack-off agent run. Add AgentChannel to the src.models import.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The channel poller and proposal-thread PI-reply poller recorded human Slack messages with message_ts set to the Slack ts but left the slack_ts / slack_channel_id mirror-mapping columns null, so "origin = Slack" reporting (and the reconcile's slack_ts dedup set) didn't see them. Content and dedup were unaffected (dedup keys on message_ts, which already equals the Slack ts here), but the mapping is now recorded for consistency with agent posts and the bulk reconcile. Web-origin reopen guidance stays null (it is DB-origin, not Slack). Verification: full suite 314 passed. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Merged main (the test-suite-characterization overhaul: tests/unit|integration| characterization|contract split, testcontainers harness, fakes/factories, CI gate). No conflicts with the refactor's src/ changes. Alignment fixups: - Move tests/test_transport.py -> tests/unit/ to match the new layout (still collected before, but keeps the convention). My edits to the three relocated files (test_message_log, test_simulation_logic, test_private_channel_migration) came across cleanly via git's rename-follow. - test_simulation_logic: add strict=False to the mint_ts zip() (ruff B905; the test lint gate is enforced by scripts/ci.sh). - test_harness_smoke: bump the pinned alembic head 0018 -> 0020 (this branch adds migrations 0019 + 0020). Verified: scripts/ci.sh passes — 460 tests (unit+integration+characterization+ contract), 13 golden-master snapshots unchanged, coverage 35.62% >= 35% floor, ruff clean; migrations 0019/0020 apply via the real alembic chain in the integration harness. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Adds tests/integration/test_pi_inbox.py (real migrated Postgres) covering src/services/pi_inbox.py — the Slack-independent path by which PI web messages and DMs enter the simulation: - get_latest_run_id picks the most recent run - record_pi_message writes a human row (is_bot=false, agent_id NULL), resolves channel_id/visibility from agent_channels, falls back to local: + public, and sets phase new_post vs thread_reply - record_pi_dm writes inbound/outbound rows; web_pi_user_id formatting Lifts src coverage 35.62% -> 35.86% (pi_inbox was 0%). Full gate: 464 passed, 13 golden-master snapshots unchanged, ruff clean. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
_flush_persisted detached the pending buffer before the try and only logged on failure, so any transient DB error at flush time silently dropped that batch. Because the DB is now the primary conversation store, a restart rebuilds from the DB and the lost messages are gone for good. Re-queue the batch on failure (prepend ahead of any entries enqueued during the failed commit, preserving chronological order) so the next tick retries. The no-DB path still clears the buffer to avoid unbounded growth. Adds regression tests covering re-queue, ordering vs. newly-arrived entries, and the no-DB clear path. Addresses H1 from the PR #19 review (issue #18). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
mint_ts precision: f"{time.time():.6f}" lost monotonicity at the current
epoch magnitude — float64 ULP (~2.4e-7s) is coarser than the 1e-6s step,
so consecutive ids could format to the same string (breaking uniqueness)
or fail to increase once parsed back to float (breaking posted_at order).
Replace with a shared TsMinter (src/agent/ids.py) that carries a monotonic
integer-microsecond high-water mark and formats to a string only at the
end, never round-tripping the fraction through a float.
M1 (canonical-id collision across processes):
- Share one mint_local_ts() helper across all writers (PI web inbox,
GrantBot) so they inherit the engine minter's per-process monotonic,
unique guarantee instead of raw time.time() (also DRYs the id format).
- M1a: add a WHERE guard to the engine flush's ON CONFLICT DO UPDATE so a
bot message can never overwrite an existing human (PI) row on a
cross-process ts collision; legitimate bot re-flush / human re-flush /
slack-mirror updates still apply.
- M1b: guard the web post route against the IntegrityError a collision
raises — roll back and retry once with a fresh id, else return 409
instead of a raw 500.
Tests: TsMinter/mint_local_ts unit tests (monotonic, unique, float-ordered,
ts-shaped, seed_floor); rewrite the mint_ts tests as precision regression
guards; integration tests for the M1a upsert guard (block bot-over-human,
allow bot re-flush, allow human re-flush) run against real Postgres.
conftest gains an optional TEST_DATABASE_URL to point the suite at an
existing Postgres when a Docker socket for testcontainers isn't available.
Addresses M1 and the mint-precision issue from the PR #19 review (issue #18).
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Both DB inbox pollers filtered posted_at > cursor and advanced the cursor to the max posted_at seen. But posted_at is stamped at row *creation* (mint time), not commit — so a row written by another process (a PI web message) can become visible only after this process has already advanced its cursor past that timestamp. That row lands below the high-water mark and is never ingested: silently, permanently, at debug log level. With Slack off this is the only PI input path, so the message is simply lost. Fix: query a lookback window behind the cursor and dedup by identity, so a late-committing row is re-queried within the window and ingested exactly once. The channel poller already deduped via the message log; the DM poller gains a seen-set (ts -> posted_at), pruned to the lookback window each poll, and _seed_pi_dm_cursor seeds it so a restart's first re-scan doesn't replay history through handle_dm. Polls are LLM-paced, so the re-scan is cheap. Also raise the three Slack-off poll-failure logs from debug to warning so a persistently failing poll isn't invisible. Tests (real Postgres): channel + DM pollers ingest a row committed below the cursor; the DM lookback re-scan dedups (processed once); the re-scan is bounded (a row older than the window is not re-queried); restart seed prevents DM replay. Addresses H2 from the PR #19 review (issue #18). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
B1 — _flush_persisted ran a full COUNT(*) over the run's agent_messages on every flush (~every tick) just to refresh the cosmetic total_messages / total_api_calls shown in the admin UI. Throttle that refresh to at most once per RUN_STATS_UPDATE_INTERVAL (30s), forced on shutdown, while still upserting the message rows every flush. total_messages may lag by a few seconds between refreshes — fine for a display counter. B2 — _rebuild_state_from_db loaded *every* row of a run (with content) into memory at startup, so restart time and RAM grew with all-time history. Bound the load to a window: messages from the last REBUILD_WINDOW_S (14d) plus the full history of any thread with no ThreadDecision (still undecided). Active-thread reconstruction only needs undecided threads, and all other restored state (proposals, prior threads, api counts) comes from DB tables, not the log — so old closed-thread bodies are safe to leave in the DB. Add _hydrate_thread_from_db to pull a specific thread back on demand, and call it in the three reopen paths (PI web reply, PI DM/Slack reopen via _reopen_thread, and rating=0 proposal reopen) so reopening an old closed thread still resolves participants and the reply-budget offset. Tests (real Postgres): stats count is throttled and force-refreshes on shutdown; rebuild loads recent + undecided threads and windows out old closed ones; on-demand hydration restores a windowed-out thread and is idempotent. Addresses B1/B2 from the PR #19 review (issue #18). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The engine seeded the channel name->id lookup by poking each client's private `_channel_name_to_id` attribute directly (simulation.py). That attribute isn't part of the Transport Protocol, so a new backend that satisfies the *declared* contract would crash with AttributeError in _ensure_seeded_channels — NullTransport only carried the attribute "for parity", which was the tell. Add a public `cache_channel_ids(mapping)` to the Transport Protocol, implement it on NullTransport and AgentSlackClient, and switch the engine's two seeding sites to call it. The write path is now part of the contract. Also drop `set_visibility_lookup`, which had zero callers across the repo (the visibility lookup is set via the AgentSlackClient constructor): removed from NullTransport, AgentSlackClient, and the test fake (plus the fake's unused _visibility_lookup attribute). Tests: cache_channel_ids seeds the lookup; it's declared on the Protocol; a Protocol-only backend without _channel_name_to_id can be seeded; the dead setter is gone. Addresses M2 from the PR #19 review (issue #18). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Fixes the three residual findings in .notes/db-conversations-residual-2026-07-24.md
— the failure modes left after H1/H2/M1a, all at boundaries the Slack-primary
system got for free (a globally unique authoritative ts, durability the instant a
message was posted, one clock).
R2 — messages lost on a hard kill
Posts are buffered in MessageLog and written to Postgres once per turn, so the
documented `docker rm -f agent-run` (SIGKILL) discarded the in-flight turn
permanently: the DB, not Slack, is the durable store. Three parts, because the
doc change alone was not enough:
* restart recipe is now `docker stop -t 30 agent-run && docker rm agent-run`
(CLAUDE.md, README.md, provision_slack_bots.py), and the agent service
carries stop_grace_period: 30s so the container's StopTimeout matches;
* the graceful path was itself racy — the handler did
`asyncio.ensure_future(sim_engine.stop())`, so the main loop could return
first and asyncio.run cancel the orphan flush mid-await. The handler is now
the sync request_stop() (flag only) and `await sim_engine.stop()` runs in the
entry point's finally-block, on every exit path;
* SIGTERM could not be noticed in time — the idle backoff sleeps up to 30s,
longer than the old 10s grace. Those sleeps now go through _sleep(), which
races the sleep against a stop event.
An in-flight LLM call is still not interruptible, hence -t 30 over the default.
R1 — cross-process canonical-id collision silently dropped a message
TsMinter guaranteed uniqueness per process, but three processes mint into the
same run (engine, web app, GrantBot). Two minting in the same microsecond
produced the identical id; the uq_agent_messages_run_ts conflict handler then
resolved it by keeping one row and dropping the other. Each minter now owns a
residue class: ids are quantized to a 100us slot with the writer id in the low
microsecond digits, so a cross-writer collision is structurally impossible. Ids
stay ts-shaped, float-parseable and strictly ordered; the cost is resolution
(one id per writer per 100us), far above the real posting rate. Writer ids are
claimed at process entry via set_default_writer_id(); the engine process claims
two, since its DM writes use the module-default minter.
Chosen over re-mint-on-conflict, which for a thread root would have to rewrite
MessageLog._by_ts, every child's thread_ts, PostRef.post_id,
ThreadState.thread_id, ProposalRef.thread_id and the ThreadDecision rows; and
over a DB sequence, which mint_ts() cannot await. The M1a guard and the web
route's M1b retry stay as backstops.
R3 — inbound delivery depended on the writers' clocks agreeing
Both DB inbox pollers paged over posted_at, which is float(the writing process's
minted ts). A PI message stamped more than the 300s lookback below the engine's
cursor was skipped silently and forever — safe on one host, not across hosts.
They now page over created_at (server_default=now(), the single Postgres
server's clock), so the window depends on one clock. posted_at remains the
ordering key for conversation content; it is just no longer the delivery cursor.
Cursors are datetimes, seeded from MAX(created_at) over the whole run rather
than the windowed rebuild's loaded rows. An id-based cursor was rejected: the PK
is a random uuid4, so it would have needed a new sequence column and a backfill.
The lookback stays — it covers commit visibility, not skew (H2).
Migration 0021 adds ix_agent_messages_run_created and
ix_pi_dm_run_direction_created to back the new access path.
Tests: writer-slot disjointness at a frozen clock (unit) and against the real
unique constraint (integration); graceful-shutdown flag/sleep/flush behaviour;
delivery of a row from a writer three years of skew behind. The H2 tests now
derive their cursor from each row's own created_at, so they no longer depend on
the test process's clock either. Full suite green: 500 passed.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The two remaining "minor observations" from
.notes/db-conversations-residual-2026-07-24.md.
Thread history was not posted_at-ordered
MessageLog.get_thread_history returned root + replies in log-insertion order.
Single-process appends are roughly ordered, but the DB inbound poller and the
Slack reconcile append entries whose posted_at predates messages already in the
log, so the LLM could get a subtly scrambled thread — unlike the Slack-primary
rebuild, which fetched in ts order. Replies are now sorted by posted_at; the
sort is stable, so entries sharing a timestamp keep their insertion order. The
root stays pinned first: it is the thread's parent by definition even when a
writer running behind stamps a reply below it. This also re-orders the input to
get_thread_allowed_agents' first-two-participants rule, which is the intended
correction.
Enabling Slack mid-conversation corrupted the mirror
Slack threads on the *root's* ts, which equals the canonical thread_ts only when
the root was born on Slack. A thread started with Slack off has a minted root id
that Slack has never seen. The finding named the recorded slack_thread_ts
column, but the defect was wider: _post_message passed the canonical thread_ts
straight to client.post_message, so the Slack API call itself was wrong and the
reply would detach or error. Fixing only the column would have left that intact.
* new _slack_parent_ts() translates canonical -> Slack via the root entry's
slack_ts, falling back to the canonical id when the root is not in the log
(windowed out by the B2 rebuild bound), which preserves pure-Slack-on
behaviour where the two are the same value;
* a reply whose root has no Slack presence is kept DB-only with a warning
rather than posted against an unknown id — the message still drives the
simulation, only the mirror skips it. Mid-life toggling stays unsupported
by design, but now degrades safely instead of corrupting the thread;
* LogEntry gains slack_thread_ts, set at every Slack-origin construction site
and written by _flush_persisted in place of the canonical thread_ts.
Prerequisite, found while fixing the above: the rebuild and hydrate paths were
dropping slack_ts from restored entries entirely, so after any restart every
root looked DB-origin. Left as-is, the new check would have been a regression
that stopped mirroring *all* replies on the first restart — and a restart with
Slack newly enabled is precisely the mid-life-toggle scenario. Both paths now
restore the mirror mapping, inferring it for pre-Stage-6 rows: a real Slack
channel_id with a NULL slack_ts means the canonical id IS the Slack ts.
No migration — slack_thread_ts already exists on agent_messages; only what gets
written into it changed.
Left alone: MessageLog.latest_timestamp has the same insertion-order flaw but
has no callers in src/ or tests/, so it is noted rather than changed. Fix or
delete it before anything uses it as a cursor.
Full suite green: 512 passed.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Two defects in private_channels.py, both cases of the DB not actually being
the primary store yet.
R4 — the Slack-off migration minted its canonical ids by hand as
f"{time.time():.6f}", the one site the R1 writer-slot fix missed. Those ids
carry no writer slot, so they can coincide with an id minted by the engine or
GrantBot; because these rows go in through the ORM rather than the ON CONFLICT
upsert, the collision aborts the whole migration transaction (channel,
members, handover, close marker) rather than dropping one message. The format
also round-trips microseconds through a float, which cannot hold them at
current epoch magnitudes, and it ignored the minter's high-water mark.
R5 — the Slack-on migration posted the handover and the origin-thread close
marker to Slack and never wrote them to agent_messages. Nothing was lost while
Slack stayed on (the reconcile pass re-reads Slack history), but a Slack-off
restart rebuilt the refinement channel with no handover in it: two bots in a
channel whose purpose is stated only in messages the DB never saw. The marker
was also posted with thread_ts=thread_decision.thread_id — the *canonical* id,
which is not a valid Slack ts once a thread has started Slack-off. That is the
same bug a93d136 fixed in the engine, still live here because the web process
has no MessageLog to resolve the root against.
Both paths now write through one helper, _add_handover_message: canonical id =
Slack ts when the mirror post landed, else minted; slack_* columns only when it
landed. _slack_parent_ts_from_db is the DB-side twin of the engine's resolver,
with the same pre-Stage-6 inference, and the marker stays DB-only (with a
warning) when the root has no Slack presence.
Two incidentals in the lines being touched: _latest_simulation_run_id moved
above the Slack side-effects, so a run-less DB fails before creating an orphan
Slack channel instead of after; and ThreadNotFound on the marker post no longer
aborts a PI-initiated migration.
Tests: three integration tests, each verified to fail against the pre-fix code
(IntegrityError on the duplicate message_ts; no rows written; a minted id handed
to Slack as thread_ts). FakeSlackClient gains _resolve_channel_id.
Full gate green: 515 passed, coverage 38.63%.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The log is append-only in insertion order, which is not time order: the DB inbound poller and the Slack reconcile append entries whose posted_at predates messages already stored. a93d136 fixed get_thread_history; three sibling reads were still keying on list position. - latest_timestamp returned _entries[-1].posted_at, so a cursor taken from it could move backwards. It has no callers, which is why it was left open — it now reads an O(1) high-water mark maintained by a new _record(), the single add path behind append/load_entry. - get_agent_top_level_posts sliced posts[-limit:] in insertion order. Callers: the Phase 5 dedup context (limit=10) and the daily post cap (limit=100). A late append of older history pushes a genuinely recent post out of the slice, so the LLM loses its most relevant dedup context and the cap under-counts. Now sorted by posted_at (stable) before slicing. - get_last_bot_sender_in_channel scanned reversed(_entries). Callers: the collab_private turn-taking gate (three sites). A late-appended *older* message answered as "the last poster", handing the turn to the wrong bot — letting one post back-to-back, or blocking the one whose turn it was. Now a max-scan on posted_at, ties resolved to the later insertion so behaviour is unchanged when timestamps collide. Tests: TestOrderingIsByPostedAtNotInsertion, 7 cases. The three behavioural ones fail against the pre-fix code (backwards cursor; newest post dropped from the slice; 'wiseman' != 'su' for the last poster); the other four pin invariants the fix must not break. Full gate green: 522 passed, coverage 38.71%. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The channel poller's bot branch built its LogEntry without slack_ts, slack_channel_id or slack_thread_ts, while the human branch 20 lines below sets all three from the same conversations.history response. The missing column is not the problem; _slack_parent_ts is. An entry with no slack_ts looks DB-origin, so any thread rooted at a polled bot post resolved to "no Slack root" and _post_message kept every reply DB-only. The roots this branch ingests are another workspace bot's posts — i.e. GrantBot's funding posts, whose threads are open to all agents — so the effect was that replies to a funding thread silently stopped being mirrored to Slack. Two things masked it: a restart repairs the mapping (_restored_slack_ts infers it from a real Slack channel_id), and it only bites for a root polled in during the current process's lifetime, so the window is "replies to today's funding post, before the next restart". Found by inspecting live rows after a restart — 60 of 625 rows in the current run carry slack_ts NULL, and the newest are GrantBot posts sitting in real Slack channels. Slack-origin means the canonical id IS the Slack ts, so the thread parent needs no translation, same as the reconcile and proposal-thread pollers already do. Forward-only: existing NULL rows still rely on the rebuild's inference, which handles them correctly; nothing backfills the column. Tests: test_polled_bot_message_keeps_its_slack_mapping drives the real poller with a canned GrantBot history entry and checks the in-memory mapping, _slack_parent_ts resolution, and that the mapping survives the flush. Fails against the pre-fix code (slack_ts is None). Full gate green: 523 passed, coverage 39.14%. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
_restored_slack_ts treated "a row in a real Slack channel with slack_ts NULL" as proof the message was born on Slack, and promoted its canonical id to a Slack ts. That covers pre-Stage-6 history, but it is unsound: a DB-origin message can carry a real Slack channel_id by two independent routes — a PI message from the web inbox (_resolve_channel returns the agent_channels row's id, Slack's when Slack is on) and an agent post whose mirror failed (_flush_persisted uses _channel_id_map, likewise Slack's id). Both mint a *local* id, so the inference fabricates a timestamp Slack never issued. _slack_parent_ts then hands it to chat.postMessage as a thread_ts: Slack drops the unknown parent, creates an orphan top-level post, post_message raises ThreadNotFound and the thread gets evicted. Nothing on the row separates the two cases — is_bot doesn't, since the failed-mirror case is a bot row — so the guess is removed rather than narrowed. slack_ts is now the only evidence of Slack presence; NULL means not on Slack. Legacy data is repaired instead of guessed at. scripts/backfill_slack_ts.py asks Slack via conversations.history whether each candidate ts actually exists and writes slack_ts only for confirmed ones; a failed lookup is reported as unverified rather than treated as absent, so an expired token cannot mark real Slack history as DB-origin. Dry-run by default, idempotent. Verified on the dev DB before this change landed: of 11 candidate rows Slack confirmed 10 (9 GrantBot posts, 1 polled human message) and denied 1 — a web-written PI row whose id the old inference would have promoted. The 10 are backfilled; the 1 correctly keeps NULL. _slack_parent_ts_from_db (added in 2a2e98c, which copied the inference) is fixed the same way. test_rebuild_infers_the_slack_ts_of_a_pre_stage6_row pinned the bug and is replaced by test_rebuild_never_infers_a_slack_ts_from_the_channel_id, built from the row shape that actually broke. DEPLOY ORDER: on a workspace with pre-Stage-6 history, run the backfill BEFORE deploying this — otherwise replies stop being mirrored into legacy Slack threads. Full gate green: 523 passed, coverage 39.08%. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Database as primary conversations
Brings the cohort interaction gate + reactive-priority scheduler onto main's db-primary-conversations base so the v2 spec (.notes/cohort-system-v2.md) can be implemented against the DB-native conversation layer.
Builds out .notes/cohort-system-v2.md in full on top of the merge of cohort-agent-isolation into main's db-primary base. The v1 spec and its implementation both predate the DB-primary rewrite and disagreed with each other in eleven places; this makes the documented rule and the running code the same thing. Migration (§14) - Renumber 0019_add_cohorts -> 0022_add_cohorts (down_revision 0021). Two migrations sharing revision "0019" is invisible to git and pytest: the merge is clean, every test passes, and Alembic only warns — then `upgrade head` dies on multiple heads while a targeted `upgrade <rev>` silently applies whichever duplicate sorts last and stamps the DB as fully migrated. Revision ids are assigned at merge, never at branch. - All downgrades idempotent (if_exists) so a rollback cannot wedge on an object a partially-applied upgrade never created. - scripts/ci.sh gains an offline alembic gate (single head, no duplicate ids) ahead of lint and pytest. `alembic check` is not sufficient — it reports "target database is not up to date", the wrong diagnosis, and needs a live DB. - New cohort_audit_events table: append-only, denormalised cohort_name/actor_email and no FK on cohort_id so the trail outlives both the cohort and the user. Gate semantics (§5) - cohort_default_policy: "open" (default) | "isolated". Under "open" an uncohorted agent is unrestricted, restoring the contract v1 published and the code inverted — enabling isolation with no topology defined is now a no-op instead of roster-wide silence. - Preflight refuses to run isolation that would silence everyone: forced off with an ERROR when no live roster agent has any membership, or when there is no DB handle. Counts live members, not cohorts — an empty cohort silences the roster just as completely as no cohorts at all. - The human bypass keys on is_bot, not `sender_agent_id is None`. agent_messages.agent_id is nullable, so a bot row with a NULL agent_id ingested by _poll_inbound_from_db would otherwise pass the gate as a human. Unattributable bot traffic now fails closed. - src/services/cohorts.py holds the decision logic as pure functions, so the engine and the admin preview cannot drift. Enforcement (§6) - has_new_reply_from_other is gated — the one read of eleven that was missed, and the one that let the scheduler prioritise exactly what the gate had rejected. - Every public MessageLog read carries a COHORT-GATE: GATED|UNGATED marker, with a test that fails when a new reader appears without one. - Writes are never gated: the log is shared by every agent in the process, so filtering at ingest would filter for all of them. Stated in code and in the spec. - Banked interesting_posts from non-permitted senders are pruned on resync. Private channels (§7) - A PI-created collab_private channel is exempt: an explicit human pairing outranks an admin-level grouping. Driven off the persisted LogEntry.visibility rather than the engine's in-memory channel map, so it is correct for rows ingested from another process and after a restart. Grandfathering (§8) - ThreadState.grandfathered. Open threads still get Phase 4 replies so they can conclude, but lose reactive priority. This is the normal path, not an edge case: the DB state rebuild runs before the first gate recompute, so every resumed run reconstructs its open partnerships cohort-blind. - _owes_reply skips grandfathered threads and reads through the agent's gate, so a non-cohort third party in an open funding thread cannot manufacture priority. Outbound hygiene (§9) - Mention stripping moved into _post_message, covering every outbound path instead of only Phase 5. Whole mention removed rather than de-@'d; unknown bot names left alone and logged at WARNING; per-agent strip counters. Scheduler (§10) - max_consecutive_reactive_turns default 8 -> 3. At 8, a single live pair took 24 of 27 turns. - turn_delay_seconds enforced as per-agent selection eligibility; the global asyncio.sleep that stalled Slack polling, DB ingestion and every other agent for one agent's cooldown is removed. - Reactive:proactive selection ratio logged every 100 picks. Admin (§12) — granular topology control - New agent x cohort matrix at /admin/cohorts/topology: every pair is a checkbox, column toggles, one audited save. Diffed against the cells that were rendered, so a partial or stale form can never delete a membership it did not display. - Per-agent "acts on" preview computed with the engine's own compute_gates. - Delete refused server-side while members exist, not just a disabled button. - Audit log on the detail page; every mutation writes an event. - Shared status banner stating what is actually in force, including a preflight override, and that filtering is forward-only. Provenance (§13.1) - Topology snapshots written to cohort_audit_events at run start and on every mid-run change, carrying the applied gate plus the counters the web process cannot see. Terminology (§1) - main already used "cohort" for date-bounded graph slices. Renamed those to run window (CABO_WINDOW_START, window_start_bound, window_posts) and replaced the dangling pointers to non-existent memory files. Testing - tests/unit/test_cohort_isolation.py (92 tests) relocated into the CI-gated layout and rewritten against v2; the old file asserted the inverted semantics. - tests/integration/test_cohort_admin.py (25 tests): real ASGI + Postgres + templates. - Full suite: 642 passed across unit/integration/characterization/contract against a real migrated Postgres, 13 golden-master snapshots unchanged. - Live migration audit on clones of the running database: fresh install, upgrade from the live DB's actual 0018, single head, downgrade/re-upgrade round trip, and a downgrade from a stamped-but-unapplied 0022 (the exact case that crashed the old migration). The live copi database was not modified. - An adversarial pass against this implementation found two real defects, both fixed with regressions: the preflight counted cohorts instead of live members, and the outbound whitespace tidy-up flattened indentation across the whole message, mangling code blocks and nested lists. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The previous commit's engine tests were unit-level with fake DB objects: the real engine had never run against a real database with cohorts on. That left the claim "the DB conversation interface works with cohorting" unverified, and it is the claim that matters most. tests/integration/test_cohort_engine_live.py (47 tests) — real SimulationEngine, real committed Postgres rows, NullTransport (Slack off, the configuration where the DB is the sole conversation store): - The topology matrix: 10 shapes x 2 policies x the disabled case, evaluated through the engine rather than the helper. Empty, one empty cohort, solo, one pair, two disjoint pairs, overlapping, one big cohort, a hub in every cohort, partial cohorting, and a cohort whose only member is off the roster. Plus a symmetry invariant over every shape — an asymmetric gate would let one side monologue. - _poll_inbound_from_db ingesting rows written as if by another process, proving the shared log stays complete while only the per-agent read is filtered, and that a bot row with a NULL agent_id fails closed. - The resumed-run path: _rebuild_state_from_db + _rebuild_agent_state reconstructing a thread cohort-blind, then the first recompute grandfathering it. - _record_topology_snapshot actually writing a row (it is wrapped in try/except, so a broken write would only have logged a warning), and a mid-run topology change leaving a second snapshot. - A real Phase 2 with a scripted LLM: the excluded agent's content never reaches the prompt, and when everything is filtered no LLM call is made at all — the actual saving, measured at the boundary where it either exists or doesn't. - Phase 3 not activating on a non-cohort tag but still activating for a mate. - Phase 4 still answering a grandfathered thread while it stays out of the reactive tier — §8's central promise, driven rather than asserted structurally. - _post_message stripping a cross-cohort mention, read back from the persisted row. - PI DMs reaching the agent under a maximally closed gate (that path bypasses MessageLog entirely). Verification of the tests themselves: seven mutants introduced into the gate were all killed — reverting the policy inversion (9 failures), restoring the NULL-agent_id hole (4), dropping the private-channel exemption (4), letting grandfathered threads keep reactive priority (2), weakening the preflight to a cohort count (5), ungating get_new_top_level_posts (4), and removing the outbound strip (1). Also verified live: the full suite passes with SLACK_ENABLED both false and true (689 each, 13 golden-master snapshots unchanged), and the 0022 downgrade drops cleanly with real cohort rows present (1 cohort, 2 memberships, 1 audit event) and re-upgrades. New finding, documented in the spec and the admin banner: get_settings() is lru_cached, so cohort_isolation_enabled and cohort_default_policy are read once per process. Topology edits are live within ~30s; changing the flag or the policy needs an agent-run restart. Without this an operator flips the flag, sees nothing change, and concludes the feature is broken. Two harness bugs found and fixed while writing these tests, both of which would have made a test pass vacuously: the live engine did not register the message-log persist callback (so a row-level assertion read an empty table), and a SimpleNamespace stood in for Settings (so driving a real phase hit AttributeError rather than exercising real defaults). The harness now copies the real Settings object. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Closes the gaps named in the previous report, and finds one more. Blocked, not skipped: ANTHROPIC_API_KEY is present in .env but EMPTY, so no real API call is possible. tests/integration/test_cohort_real_llm.py is written and wired instead — 4 tests, marked real_llm, skipped without a key. Verified both ways: 4 skipped with no key; with a probe key all 4 execute and reach the Anthropic API (401 invalid x-api-key), so the plumbing is live and only the key is missing. It checks what a fake cannot: that a real model's OUTPUT is unaffected by gated-out content (marker-string proof, with an ungated control), that the outbound strip works on genuine model prose, and that a real scan response can only name post ids that survived the gate. Capped at 300 max_tokens on Sonnet, ~4 calls total. Browser testing via Playwright, 20 agents, 4 cohorts, real app instance on a separately-migrated database: - The matrix renders 20 rows x 4 columns = 80 cells with exactly 80 `present` fields. - The JS column toggle — previously untested — works: one real click checked all 20 boxes in a column, a second unchecked them, adjacent columns untouched. - A 19-membership topology built in the DOM and saved with a real click on Save returned "19 added, 0 removed", landed correctly in the DB, and the per-agent "Acts on" preview computed the right unions, including the two agents in two cohorts each (8 mates apiece) and the three uncohorted agents showing "everyone (gate off)" under policy=open. - Restarting the instance with policy=isolated flipped those three to "humans + PI private channels only" and named them in the banner — which also demonstrates the lru_cache restart requirement from the other side. - Clearing the matrix through the UI produced the red "switched on but forced OFF" preflight banner with its reason and the all-vs-all reassurance. That is the exact state that would have silenced the roster under v1. Multiprocess concurrency: three real OS processes against one Postgres — one writer churning memberships, two engines recomputing gates. 598 topology rewrites against 5,813 recomputes: zero errors, zero asymmetries, zero torn reads, no hangs. p50 2.6 ms, p99 ~9 ms, max 156 ms at 20 agents. NEW FINDING from that test. The safety depends on the membership write being atomic, and the dependency is invisible. A hostile writer that commits the wipe separately from the re-insert put ~57% of concurrent recomputes into the preflight-refused state — the gate fully OPEN for those ticks — versus 0 of ~4,500 with the single-transaction writer the shipped route uses. Any future bulk importer or seeding script that truncates then inserts would silently un-gate the roster about half the time under policy=isolated, and no code review would show it. Now pinned by test_matrix_save_writes_memberships_atomically and documented as normative in v2 §6.3.1, with the note that the failure is fail-open, never fail-closed — which is the right direction and also why it is easy to miss. Scale measured (v2 §14.6): the every-30s recompute is 2.3 ms p50 at 4 cohorts / 20 memberships and still 11 ms at 400 cohorts / 2,000 memberships. Free at any plausible roster; no caching warranted. Adversarial pass 3, 14 checks, all passing: cohort membership is correctly NOT transitive (A-B and B-C must not give A-C); 20 singleton cohorts leave every agent seeing only itself and none un-gated; all-20-in-one and every-agent-in-every-cohort both collapse correctly; the DB rejects duplicate (cohort, agent) at the constraint; SQL-looking agent_ids are inert data; all 190 unordered agent pairs symmetric over a random topology; and the admin preview matches the engine gate agent-for-agent on the topology that was saved through the browser. .notes/cohort-long-run-plan.md — three phases, ~500 calls, budget-bounded rather than time-bounded: a gate-OFF baseline (without which a quieter simulation is not evidence the gate works), a gate-ON run under policy=open, and a mid-run topology change to observe grandfathering under real conversational load. Includes the five blocking prerequisites (the empty API key and the live DB being three migrations behind its own code are both on that list), measured per-turn call costs, abort criteria, and the SQL that finds any cross-cohort thread. Full suite: 691 passed, 4 skipped (the real-API tests), 13 golden-master snapshots unchanged. Live copi database untouched at 0018 throughout; all scratch databases and the UI test container removed. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
A working Anthropic key made two claims checkable that were not before — and both of the tests written for them were passing while proving nothing. Capturing the actual model output is what revealed it. Verified, not assumed: the configured model ids `claude-opus-4-6` and `claude-sonnet-4-6` are both valid and each resolved on a live call. Pricing for the cost model in the run plan: $5/$25 and $3/$15 per MTok. Vacuity #1 — the real-API test. It passed. With no profile loaded on the agent, the scan selected NOTHING in both the gated and ungated legs, so the gated assertion ("the model did not select the excluded post") held for the wrong reason and the ungated control demonstrated no difference at all. Rewritten around a profile that makes the excluded post directly relevant and the surviving post clearly irrelevant, which produces a measured behavioural difference: gate off -> model selects ["1000.0002"], reasoning cites the match explicitly gate on -> model selects [], having considered only the survivor That is the claim the feature rests on: ungated, the agent would have opened a thread and spent Opus calls on the excluded partner. A fake LLM can only show the prompt lacks the text. Documented as v2 §15.1 with the general rule — a test whose passing condition is an ABSENCE is vacuous without a paired positive leg showing the thing would otherwise be present. Vacuity #2 — the same mistake at experiment scale, in the live multi-turn run. Phase B (gate on) reported zero cross-cohort threads; so did Phase A, the gate-OFF baseline. Each cohort pair had been given matching interests and the pairs made mutually irrelevant, so even ungated the model correctly declined to pair chemistry with imaging. Redesigned: all four labs complementary on one problem (targeted protein degradation), cohorts drawn to cut across the strongest natural affinities (alpha=[su, wiseman], beta=[cravatt, lotz]), so the gate must block exactly the pairings the model most wants. Corrected two-phase run, real Opus/Sonnet calls, Slack off, one database per phase: A gate OFF: 18 msgs, 5 threads, pairs cravatt<->su, lotz<->su, lotz<->wiseman — all 3 cross-cohort, 4 SQL violations (the query catches leaks) B gate ON: 21 msgs, 4 threads, pairs cravatt<->lotz, su<->wiseman — 0 cross-cohort, 0 violations Under the gate the agents talked MORE (21 vs 18 on the same budget) but only to cohort-mates, and the two conversing pairs are exactly the two cohorts — the gate redirected collaboration rather than suppressing it, which rules out "the gate just made the simulation quieter." Verified twice: by the run script and by an independent SQL join of agent_messages to cohort_memberships. Also found: the first harness shared one database across phases, so one phase's setup deleted rows the other's still-buffering engine was about to flush. The engine re-queued those messages rather than dropping them — the H1 fix from the original audit working under a real mid-flight failure I caused by accident. Full suite: 694 passed with a key present (the 3 real-API tests execute rather than skip); 691 + 3 skipped without one. Live copi database untouched at 0018 throughout; all scratch databases dropped. The key was never written to any repo file — verified against the working tree, the git index, and git history. Not closed: Slack-on mirroring under an active gate. No bot tokens exist here (zero agents carry one, no SLACK_* value set), so every automated test uses NullTransport. The §13.1 topology snapshot is also not exercised by the run harness, which drives _recompute_allowed_sender_ids() directly rather than start(); that path is covered by test_topology_snapshot_is_actually_written. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Both were invisible to the existing suite, and both broke conversation
rather than leaking it — the gate was over-restrictive, not under.
1. compute_gates: under policy "open", "unrestricted" only held in one
direction. The uncohorted agent's own gate is None so it could act on
anyone, but every cohorted agent's gate is the union of its co-members
and so never contained it. It could open threads and never be replied
to. Uncohorted agents are now added to each cohorted agent's mate set,
which is what spec §5.1 says ("A has no cohort memberships, policy =
open -> Yes") and makes the relation symmetric. policy "isolated" is
unchanged: there, uncohorted still means excluded.
2. _post_message: never stamped `visibility`, so every agent-authored
message persisted as "public" even inside a collab_private channel.
Two readers depend on that field — the gate's private-channel
exemption (§7), which is how a PI pairing outranks an admin cohort
grouping, and the G2 memory filter that keeps private content out of
the public segment. The exemption was dead code and private content
was eligible for public memory. Now stamped from the channel class,
defaulting to public for unregistered channels.
Two of my own unit tests had pinned defect 1 (asserting the asymmetric
gate as expected) and are corrected here. The symmetry test skipped the
None-vs-set case, which is exactly how it missed it; it no longer skips.
The §7 test could not catch defect 2 because it wrote the AgentMessage
row with visibility pre-set, exercising only the read path — the new
test posts through _post_message and reads back what landed.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01YE9WxUMYvwhvfumLaC9irh
… sync (#29) _load_publication_records joins agents/publications and stores an agent_id -> LabPublicationRecord map on SimulationEngine, wired into _sync_roster_from_db right after the roster query. An agent with zero publication rows is absent from the map (fail closed — cannot verify). DB DOIs are also pushed onto Agent.db_publication_dois and unioned into own_publication_dois so the intake guard (cites_own_paper) benefits too. Also updates test_roster_sync.py's _FakeDB stub to serve a second query per session block, since _sync_roster_from_db now issues the publications join alongside the existing roster query. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…sync (#29) _load_publication_records was called inside the same try/except and session block as the AgentRegistry roster query in _sync_roster_from_db. Any exception from the publications join (e.g. a transient DB hiccup) was caught by the OUTER handler and silently no-op'd the WHOLE roster tick before the add/remove/role-diff logic ever ran — new agents didn't get added, removals didn't propagate. Wrap just the publications load in its own try/except: log a warning and continue. The previous _agent_publications map (and each Agent's db_publication_dois) is left exactly as it was on failure — stale grounding beats an aborted roster tick, and agents absent from the map still fail closed regardless. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…l-closed (#29) Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
#29) Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
) Memory synthesis was the last unguarded path where a hallucinated "Co-authored X" line could get written to disk and DB, then re-injected into every future prompt. _update_agent_memory now runs the synthesized text through strip_ungrounded_authorship_lines against the agent's own publication record (DB + profile DOIs) before it reaches update_working_memory_file and the profile-revision row, and the synthesis prompt now instructs the model to name authoring labs explicitly rather than write subject-less authorship claims. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
… line (#29) Task 5 review flagged two coverage gaps in ed734f3: - The CollectiveName branch, missing-LastName skip, and LastName+Initials concatenation were implemented but only exercised via a hand-built authors list in the unit test, never through the actual XML parser. Add a second contract fixture whose AuthorList hits all three branches through fetch_pubmed_records, asserting the skip case is absent from `authors` while still counted in `author_count`. - _execute_retrieve_full_text's new Authors line had zero test coverage. Add tests that monkeypatch fetch_full_text (mirroring the existing fetch_abstract monkeypatch) and assert the delimited Authors line appears between Title and Journal, and is omitted when there are no authors. - Add >20-author truncation tests (21 authors -> first 20 + "+1 more"; exactly 20 -> no suffix) for _execute_retrieve_abstract, the other edge case called out in review but previously untested. No src/ changes — tests only. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…se-5 prompts (#29) Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…rs (#29) Review found two gaps in the sweep: the glob *//public.md structurally can't see the legacy profiles/memory/<agent_id>.md fallback that Agent.public_working_memory still reads for unmigrated agents, and a single unreadable/malformed file aborted the whole run with a raw traceback, hiding findings already gathered from other files. sweep() now also scans top-level *.md files (excluding *.pre-sweep backups) as the legacy layout, and wraps each file's processing in try/except so one bad file is reported and skipped instead of crashing the run. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…ction (#29 audit C1, M1) The verb-anchored grammar missed every realistic paraphrase in the adversarial audit — auxiliaries ('has co-authored'), contractions ('we've'), noun forms ('we're co-authors on', 'as a co-author of', 'I was senior author on'), team/group subjects, 'behind the ... paper', 'a paper of ours', Slack *emphasis* wrapping the verb, and unicode hyphen/apostrophe variants. Text is now normalized (emphasis stripped, unicode punctuation folded) before matching, and the grammar covers all sixteen pinned probes while keeping the legit third-party negatives. Also bounds _OTHER_LAB_SUBJECT_RE's capitalized-token run at 6 tokens (audit M1): the unbounded run backtracked quadratically — ~85s at 20k tokens on the event loop, now 0.04s. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
… audit I2, I3)
claims_coauthorship now covers 'wrote/published ... (together) with',
'our joint paper with', and 'jointly authored' — the WUBOT_ORIGIN shape
reworded to dodge the literal 'co-author' stem no longer skips the
tagged-lab records check.
validate_authorship_claims no longer lets an own-DOI anywhere in the
message satisfy a claim about a different paper: a DOI grounds a claim
only from the claim's own sentence, with no intervening first-person
re-anchor ('building on our earlier BioThings work (btad570)') between
claim and DOI, or from an immediately following DOI-only sentence. The
legit own-paper share (claim and DOI in one sentence) still passes.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…author labs (#29 audit I1, I4) The phase-5 local _strip_disallowed_tags ran before the authorship gate, so a cohort-disallowed co-author's @tag was deleted before the tagged-co-author check could see it — and _post_message's chokepoint pass then also saw only the laundered text. The gate now runs on the original draft (it is read-only, so the swap is safe). _reject_ungrounded_authorship also resolves prose-named labs ('the Good lab's') through the roster — PI last name or agent_id — and enforces their records exactly like a tagged bot's. Unresolved names are left alone; same-surname collisions get the union of the namesake records. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…-person subjects (#29 audit I5) strip_ungrounded_authorship_lines now takes the agent's own identity (lab_self_names: bot name, PI name, last name). The other-lab-subject exemption is void when the line also self-refers ('with our lab', 'with us', 'we ... together') or names the agent's OWN lab/PI as the subject ('Good Lab co-authored ...' in good's own memory). The verb-line net is widened to jointly/together/bare 'wrote|authored|published ... with' forms. Both callers — _update_agent_memory and the sweep script (which now loads identities from AgentRegistry) — pass identity. Pure third-party lines with no self-reference are still kept. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…uard (#29 audit I6) _load_records was DB-only while _reject_ungrounded_authorship checks publications rows ∪ profile-parsed DOIs — so --fix deleted true memory whose only grounding was the profile ('We published BioThings Explorer (btad570)' for a lab with the DOI in its profile but 0 DB rows). _augment_with_profile_dois unions public+private profile DOIs into each record, wired into main() behind --profiles-root. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
… M2) <https://doi.org/10.x/y|our paper> yielded the DOI '10.x/y|our', which can never match a record set — silently un-grounding legit claims and corrupting claim-scoped DOI association. '|' joins the excluded delimiter class; bare, parenthesized, angle-bracketed and trailing-punctuation forms are unchanged (pinned by the existing tests). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…ll_text (#29 audit O-I3) fetch_abstract now returns the article-scoped DOI, and both retrieve executors cite it as a SEC-14-delimited 'DOI:' line (tag paper_doi, omitted when PubMed has none). Without this, the emit gate's cite-a-DOI requirement was unsatisfiable for a legit first-person share: the model had no verifiable identifier to cite. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…ackoff (#29 audit O-I4) The phase gates + backoff counters are the only loop-breaker against a model that keeps regenerating the same ungrounded draft, and they were unpinned. Two new tests drive the REAL code paths: _reply_to_thread twice (authorship_reject_count reaches 2, has_pending_reply flips False, nothing posted) and _phase5_new_post (consecutive_phase5_skips increments, nothing posted). Mutation-checked: neutering either gate call fails the corresponding test. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…s; muted-labs note (#29 audit O-runbook) Reorders the rollout so the memory sweep runs AFTER the old agent-run is stopped and removed — the running old code caches the poisoned memory in-process and writes it back over the cleaned file on the next synthesis. Fixes the verification grep to the strings the code actually logs (Rejected draft / Rejected reply to thread / Suppressed post to / stripped ungrounded), replaces the meaningless roster-sync check with the absence of 'publication-record load failed' plus a per-agent rejection-rate sanity check, and documents the eleven zero-publication labs that stay muted for first-person paper claims until the DOI exposure change and a publications backfill land. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
This was referenced Aug 11, 2026
Open
Open
d4ee439 moved the default llm_agent_model from claude-sonnet-4-6 to claude-opus-5 along with the opus knob, which silently repriced every default-model call — phase-2 scan, phase-2 prune, memory synthesis, and the make_decision helper — onto Opus ($5/$25 vs $3/$15 per MTok). Those are the highest-volume calls in the loop and were never meant to move. The default is now claude-sonnet-5, cost-matching the prior Sonnet tier (same sticker; Sonnet 5's tokenizer spends ~30% more tokens on the same text, partly offset by intro pricing through 2026-08-31). Phase 4/5 keep claude-opus-5 via llm_agent_model_opus. The thinking={"type":"disabled"} pin carries over unchanged — Sonnet 5 also thinks by default when the param is omitted, and the tight snapshot-pinned per-phase caps rely on thinking output not competing for max_tokens. tests/unit/test_model_tiering.py pins the tiering: the config values, the model a model-less generate_agent_response/make_decision call lands on, and the thinking pin surviving the change. Co-Authored-By: Claude Fable 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
3 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.
Closes #29 (targets
copi-prod, stacked on theemail-fixline).Fixes the false-positive authorship bug from issue #29:
GoodBotpublicly claimed its lab co-authored Desiderata for a biomedical knowledge network (10.1093/bioadv/vbag036), a paper Ben Good is not an author on. The RCA (issue comment) traced it to a six-week chain — WuBot invented the claim, GoodBot agreed, memory synthesis laundered it into a durable note, and it re-emitted as a:newspaper:post.What this does (layered, deterministic-first)
src/agent/authorship_rules.py): detect first-person authorship claims and validate them against publication records, failing closed — a lab with zero records can never emit, confirm, or durably remember a first-person authorship claim. Co-authorship claims are checked against the tagged/named co-author's records too (this is what catches the WuBot-origin case, where the DOI was genuinely the sender's but the claimed co-author was fabricated)._load_publication_records), unioned with profile-parsed DOIs; the load is isolated so a query failure can't abort the roster tick._reject_ungrounded_authorship: phase-4 reply (with reject-and-backoff), phase-5 post (before tag-strip), and the_post_messagechokepoint (defense in depth).Verification
./scripts/ci.shgreen: 1775 passed / 120 skipped, 67.40% branch coverage, ruff 256/260. Regression tests pin the verbatim incident / origin / poisoned-memory texts from prod forensics.Rollout — read
docs/issue-29-remediation.mdbefore deployingOrder matters: stop/remove the old
agent-runbefore the sweep (the running old code re-writes its poisoned in-process memory cache back to disk otherwise), then rebuild the baked agent image, sweep, and restart. 11 active labs currently have zeropublicationsrows (badran, cravatt, good, kern, lotz, maillie, pwu, saez, schultz, williamson, wilson) and are muted for self-attributed paper claims until backfilled — the DOI-exposure change plus a publications backfill are the prerequisites.Known conservative-direction follow-ups (non-blocking, no failure mode reopened)
_fails closed under normalization (rare for journal DOIs).🤖 Generated with Claude Code