fix(events): give agent events distinct types instead of collapsing them (#1116)

[frankbria]

Closes #1116.

The classifier was one substring test

AGENT_STEP_STARTED if "started" in event_type else AGENT_STEP_COMPLETED

Everything that was not "started" was "completed" — 192 AGENT_STEP_COMPLETED
console lines in one run, three of them for the same path in the same second
representing three different events.

The noise is not the worst part. verification_failed, execution_failed
and step_failed were all reported as AGENT_STEP_COMPLETED — a failure showing
up as a success in the event log docs/QUICKSTART.md tells users to consult when
a run fails. There is now a test asserting no known failure event can read as a
completion.

Suffix classification, not a lookup table

The agent vocabulary is consistently <thing>_<outcome>:

step_started        planning_started       verification_started   self_correction_started
step_completed      planning_completed     iterations_completed   already_completed
step_skipped
step_failed         execution_failed       verification_failed    stall_failed
blocker_created     escalation_blocker_created

So classifying on the outcome suffix gets all 20 right, and a new event name
classifies correctly without anyone remembering to update a table — which is the
failure mode a hard-coded map would have. blocker_created and
escalation_blocker_created are explicit overrides onto the existing
BLOCKER_CREATED.

Unmapped names get the new AGENT_EVENT. An unknown event is not a completed
one, and pretending otherwise is exactly what hid the failures.

The console fix was elsewhere

The real type was in the payload as agent_event all along. print_event
whitelists which payload keys it renders and that key was not in the list, so the
terminal genuinely could not distinguish the lines. Adding it makes the reported
output readable:

06:58:17 AGENT_STEP_SKIPPED   agent_event=step_skipped path=todo_api/database.py status=SKIPPED

Acceptance criteria

• Distinct EventTypes (started / completed / skipped / failed), not a substring test
• The console line shows the actual agent event type
• Unmapped types get a distinct type rather than folding into completed
• A test asserts a skipped step and a completed step emit different types

30 tests. ruff clean. Full suite: 6381 passed, 49 skipped — nothing
depended on the old collapse.

Note

AGENT_STEP_SKIPPED, AGENT_STEP_FAILED and AGENT_EVENT are new values on the
EventType constant class. They are additive — no existing value changed meaning
— but anything consuming the event stream and matching on AGENT_STEP_COMPLETED
will now see fewer of them and some new names. That is the point of the change,
and the web UI reads event_type generically rather than matching these
specific values.

[github-actions]

Claude finished @frankbria's task in 5m 10s —— View job

---

GLM precision bug-hunt review — PR #1136

Done.

Scope: logic errors, crash/null paths, security, data loss/races, breaking contract changes, resource leaks. (Style, architecture, naming, coverage are covered by the other reviewer.)

✅ GLM review: no defects found.

What I verified against the checked-out code, not just the diff:

Area	Verdict
Suffix-classifier logic (runtime.py:1144)	Order-independent — none of _skipped/_failed/_started/_completed is a terminal substring of another; None/"" → AGENT_EVENT via (event_type or "").lower(); case-insensitive.
getattr indirection (runtime.py:1155,1158)	Every attr name (AGENT_STEP_SKIPPED/FAILED/STARTED/COMPLETED, BLOCKER_CREATED, AGENT_EVENT) exists on EventType — no AttributeError path.
Contract change (fewer AGENT_STEP_COMPLETED, new names)	Safe — grepped every event_type ==/in consumer in source; none match AGENT_STEP_* or the new values (hits are only in test transcripts + this test). Web UI reads event_type generically (EventItem.tsx only special-cases heartbeat).
blocker_created override → emits real BLOCKER_CREATED	No double-fire: emit_for_workspace (events.py:207) only INSERTs + prints; the webhook fires directly from core/blockers.py:179, not via event-type subscription. And no current adapter emits those names, so the override is dead-but-harmless.
No failure reads as success	Every *_failed name → AGENT_STEP_FAILED; non-suffix names → AGENT_EVENT, never AGENT_STEP_COMPLETED.
print_event whitelist (events.py:366)	Adding agent_event is purely additive.

The one behavioral change worth naming (not a defect): names that previously collapsed to AGENT_STEP_COMPLETED because they lack started — e.g. verification_passed, tool_call, output, error — now land on AGENT_EVENT instead. That is the intended design ("an unknown event is not a completed one"), and since no consumer matches these types it only changes the console label, with the true name still shown via the agent_event= payload field.

[claude]

Review

Good fix for the core bug: step_failed/execution_failed/verification_failed no longer read as AGENT_STEP_COMPLETED, the new suffix-based classifier is simple and well-tested for the vocabulary it targets, and the agent_event console fix is a nice, minimal catch. Also confirmed no other code paths pattern-match on AGENT_STEP_COMPLETED/AGENT_STEP_STARTED for control flow, so the behavior change is low-risk there.

However, the classifier misses part of the real vocabulary emitted by the legacy agent.py (--engine plan), which weakens the "gets all 20 right" / "nothing depended on the old collapse" claims:

1. verification_error (agent.py:926) still doesn't classify as a failure.
This is emitted from an except branch during verification — the same failure class as verification_failed, which the PR explicitly calls out as one of the three offenders that used to falsely read as COMPLETED. But verification_error ends in _error, not _failed, so _agent_event_to_event_type falls through to the new AGENT_EVENT bucket instead of AGENT_STEP_FAILED. It no longer falsely reads as completed (the literal acceptance criterion), but it also doesn't read as failed — so a real failure signal is still not visible as one in the event log, which is the actual problem this PR set out to fix.

2. agent_finished (agent.py:398, 439) — the plan engine's terminal event on every run — now degrades to generic AGENT_EVENT.
Before: "started" not in "agent_finished" → AGENT_STEP_COMPLETED (wrong when the run failed, but at least specific). After: no suffix matches _finished, so it becomes AGENT_EVENT regardless of whether the run succeeded, failed, or was blocked (the real outcome is only in data["status"], which _agent_event_to_event_type doesn't look at). This is the terminal event for the plan engine's most important signal, and it's untested — the new test file covers agent_completed (which is never actually emitted; grep shows only agent_finished is) but not agent_finished itself.

Both of these are pre-existing agent.py event names (not part of this PR's diff), verified via grep -n '_emit_event("' codeframe/core/agent.py, which also turns up several other names that will newly fall into AGENT_EVENT (agent_resumed, execution_aborted, cost_cap_exceeded, tactical_resolved, technical_fix_needed, existing_blocker, loading_context, verification_passed). Most of those are a genuine improvement (they shouldn't have been "completed" either), but verification_error and agent_finished specifically are failure/outcome-bearing events that deserve AGENT_STEP_FAILED (or, for agent_finished, a status-aware mapping) rather than the generic fallback — worth a follow-up (or amending _AGENT_EVENT_SUFFIXES/_AGENT_EVENT_OVERRIDES to add _error as a failure suffix and map agent_finished off data["status"]).

Other notes

• codeframe/core/events.py and runtime.py changes are otherwise clean; print_event's new agent_event key addition is exactly the minimal fix described.
• Test file is well-organized and the "no known failure event reads as completed" test is a good regression guard, though per above it doesn't cover the full emitted vocabulary (verification_error, agent_finished aren't parametrized).
• No security or performance concerns — this is pure event-classification logic, no new I/O or external input handling.

Nothing here blocks the merge — the PR is a clear improvement over the substring test it replaces — but the two gaps above mean the "no failure event reads as completed" guarantee isn't quite as complete as the PR description claims, and verification_error/agent_finished seem worth a quick follow-up.