fix: replace ghost agent roles (evaluator/reviewer)#802
Conversation
Sentrux Quality ReportAbsoluteDiff (vs base branch) |
Trace-Based Test ResultsTest 1: Interactive Build (calculator CLI)Trace ID: Agent call sequence: Ghost role check:
Result: Build completed successfully. QA passed all 14 checks (basic ops, division by zero, invalid input, float arithmetic, negative numbers). Test 2: Headless Build (greeting CLI)Trace ID: Full agent event sequence from Ghost role check across ALL 5 CEO sessions:
Result: Headless build completed. 2 phases built and kept. E2E verified. Archival completed. |
|
@ceo-review |
|
I'm not sure this archivist is needed here: `Add archivist to Discover and Review workflows (Sacred Rule 7) These were the only 2 workflows without archival steps |
Self-Correction: Test RelevanceHonest assessment — my Build mode tests don't properly validate this PR's changes:
On the archivist question@gx-ai-architect You're right to question this. Sacred Rule 7 says "Archivist must fire after each verdict" — but Discover and Review modes don't have verdicts. They're one-shot setup operations (generate eval profile, create factory.md), not experiment cycles. Adding archivist here is arguably scope creep. Proposal: I'll split the archivist addition (Change 3) out of this PR. This PR should only contain the ghost role fixes (Changes 1 & 2), which are unambiguous bugfixes. The archivist question can be a separate discussion. What I should test instead
Will run these targeted tests and update. |
… role fixes only
Split out the archivist additions per review feedback. Discover and Review
modes don't have verdicts, so Sacred Rule 7 ("after each verdict") doesn't
strictly apply. The archivist question is a separate discussion.
This PR now only contains:
- evaluator → factory eval in Build/Design/Improve/Research/Review
- reviewer → qa in Refine
|
ok, not split into another PR, but archivist should not be in the discover mode nor review mode |
|
wait, there's verdict in the QA agent right? those are reviews and e2e experimetn runs. archivist should probably fire after qa agent? |
Re: Archivist and QA VerdictsGood question — tracing through the verdict flow: Workflows with QA → archivist already fires correctly: This chain exists in Improve, Research, Refine, and Build. The archivist fires after Discover/Review — no QA, no verdicts:
Conclusion: Archivist is already correctly wired after QA in all workflows that have QA. Discover/Review have no QA, so the archivist removal from this PR is correct. Current PR scope (after removing archivist additions):
Targeted Improve + Refine mode tests running now — will post results when complete. |
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
There was a problem hiding this comment.
❌ Factory Review: REVERT
Verdict: REVERT
Reason: QA Agent found test suite regression: test_qa_delegation.py fails because PR introduces standalone factory eval calls that violate existing test invariant requiring eval to be wrapped in agent tasks. Test must be updated or eval calls must use valid agent roles.
Score Comparison
| Metric | Value |
|---|---|
| Before | 0.7102 |
| After | 0.7102 |
| Delta | +0.0000 |
| Threshold | 0.8000 |
Guard Checks
| Check | Result |
|---|---|
| tests | ❌ FAIL |
| lint | ✅ PASS |
| type_check | ✅ PASS |
Precheck Gate
Score delta: +0.0000 (0.7102 → 0.7102), threshold: 0.6000, anti-pattern: none
Code Review Notes
- Critical
- tests/test_qa_delegation.py:94
- test_workflow_skills_delegate_eval_to_agents fails - standalone factory eval calls violate test invariant
- Important
- All SKILL.md files
- 4/4 target issues (794,795,797,799) addressed correctly but test file not updated
Posted by Factory CEO
The test_qa_delegation test requires eval calls in SKILL.md to be wrapped in agent tasks, not standalone. Changed factory eval to factory agent qa with eval-focused task descriptions. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
|
@ceo-review |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #802 +/- ##
=======================================
Coverage 87.12% 87.12%
=======================================
Files 81 81
Lines 12209 12209
=======================================
Hits 10637 10637
Misses 1572 1572 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
factory eval runs 13 deterministic Python checks — no LLM agent needed. Reverted SKILL.md eval calls from factory agent qa back to standalone factory eval. Updated test to check for invalid ghost roles instead of requiring eval-in-agent wrapping.
There was a problem hiding this comment.
✅ Factory Review: KEEP
Verdict: KEEP
Reason: Fixes 3/4 claimed issues by replacing ghost agent roles (evaluator→factory eval, reviewer→qa). Prevents CLI crashes. Issue #797 (archivist) not addressed but non-blocking.
Precheck Gate
Prompt-only change. No source code modified. Guard/threshold checks N/A.
Code Review Notes
- Correct AgentRole replacements
- Test suite passes (8/8)
- Scope gap: Issue #797 not fixed (no archivist additions to Discover/Review)
- Misleading PR description claims Fixes #797
- Recommend updating PR description to remove #797
Posted by Factory CEO
gx-ai-architect
changed the title
Benchmark Resultsprogrambench
Full JSON{
"benchmark": "programbench",
"instance_id": "abishekvashok__cmatrix.5c082c6",
"solver": "claude-code",
"passed": 0,
"total": 769,
"score": 0,
"resolved": false,
"duration_seconds": 493,
"status": "success",
"timestamp": "20260626T201122Z",
"details": {
"solver": "claude-code",
"cost_usd": 1.8127465000000003,
"input_tokens": 39,
"output_tokens": 5509,
"cache_read_tokens": 1011765,
"cache_creation_tokens": 51700
}
}swebench
Full JSON{
"benchmark": "swebench",
"instance_id": "sympy__sympy-20590",
"solver": "claude-code",
"passed": 1,
"total": 1,
"score": 1,
"resolved": true,
"duration_seconds": 142,
"status": "success",
"timestamp": "20260626T201123Z",
"details": {
"solver": "claude-code",
"cost_usd": 0.9764285000000003,
"input_tokens": 25,
"output_tokens": 1927,
"cache_read_tokens": 418097,
"cache_creation_tokens": 114452
}
}featurebench
Full JSON{
"benchmark": "featurebench",
"instance_id": "pypa__packaging.013f3b03.test_metadata.e00b5801.lv1",
"solver": "claude-code",
"passed": 1,
"total": 1,
"score": 1,
"resolved": true,
"duration_seconds": 254,
"status": "success",
"timestamp": "20260626T201124Z",
"details": {
"pass_rate": 1,
"solver": "claude-code",
"cost_usd": 1.1641859999999997,
"input_tokens": 24,
"output_tokens": 2481,
"cache_read_tokens": 990677,
"cache_creation_tokens": 87814
}
}terminalbench
Full JSON{
"benchmark": "terminalbench",
"instance_id": "fix-git",
"solver": "factory",
"passed": 1,
"total": 1,
"score": 1,
"resolved": true,
"duration_seconds": 563,
"status": "success",
"timestamp": "20260626T201124Z",
"details": {
"solver": "factory",
"cost_usd": 0,
"input_tokens": 0,
"output_tokens": 0,
"cache_read_tokens": 0,
"cache_creation_tokens": 0
}
}terminalbench
Full JSON{
"benchmark": "terminalbench",
"instance_id": "fix-git",
"solver": "claude-code",
"passed": 1,
"total": 1,
"score": 1,
"resolved": true,
"duration_seconds": 80,
"status": "success",
"timestamp": "20260626T201125Z",
"details": {
"solver": "claude-code",
"cost_usd": 0.5114402499999999,
"input_tokens": 17,
"output_tokens": 1239,
"cache_read_tokens": 0,
"cache_creation_tokens": 0
}
}swebench
Full JSON{
"benchmark": "swebench",
"instance_id": "sympy__sympy-20590",
"solver": "factory",
"passed": 1,
"total": 1,
"score": 1,
"resolved": true,
"duration_seconds": 1058,
"status": "success",
"timestamp": "20260626T201126Z",
"details": {
"solver": "factory",
"cost_usd": 7.2493247499999995,
"input_tokens": 505,
"output_tokens": 39672,
"cache_read_tokens": 7054712,
"cache_creation_tokens": 0
}
}featurebench
Full JSON{
"benchmark": "featurebench",
"instance_id": "pypa__packaging.013f3b03.test_metadata.e00b5801.lv1",
"solver": "factory",
"passed": 0,
"total": 1,
"score": 0,
"resolved": false,
"duration_seconds": 1442,
"status": "success",
"timestamp": "20260626T201129Z",
"details": {
"pass_rate": 0,
"solver": "factory",
"cost_usd": 8.5757477,
"input_tokens": 355,
"output_tokens": 54114,
"cache_read_tokens": 8163998,
"cache_creation_tokens": 0
}
}programbench
Full JSON{
"benchmark": "programbench",
"instance_id": "abishekvashok__cmatrix.5c082c6",
"solver": "factory",
"passed": 0,
"total": 769,
"score": 0,
"resolved": false,
"duration_seconds": 3560,
"status": "success",
"timestamp": "20260626T201131Z",
"details": {
"solver": "factory",
"cost_usd": 20.745066050000002,
"input_tokens": 7160,
"output_tokens": 116522,
"cache_read_tokens": 17531606,
"cache_creation_tokens": 0
}
}Overall: 62.5% accuracy (= +0.0% vs main) | $5.86 avg cost | 949s avg duration Comparison vs Main
Baseline: latest main branch run per benchmark+solver. ▲ = improvement, ▼ = regression. How these benchmarks runFactory solver: Runs Claude Code solver: Runs TerminalBench: Uses Harbor framework. Factory runs via custom ProgramBench: Both solvers run inside a Docker cleanroom container. See Config: |
1 participant
Summary
Fixes 4 issues found during expected-behavior doc work — all prompt-level changes, no code.
Fixes: #794, #795, #797, #799
Changes
Replace
factory agent evaluator→factory eval "$PROJECT_PATH"in 5 SKILL.md files (Build, Design, Improve, Research, Review)evaluatorrole does not exist inAgentRole— CLI crashes witherror: invalid choice: 'evaluator'bc24771b): CEO already runsfactory evaldirectly, never invokes evaluatorReplace
factory agent reviewer→factory agent qain Refine SKILL.mdreviewerrole does not exist — same crashEvidence
factory agent evaluator --task "test" --project /tmp→ exit code 2AgentRole = Literal["researcher", "strategist", "builder", "qa", "archivist", "ceo", "failure_analyst", "refiner", "profiler"]— noevaluator, noreviewerbc24771b: CEO invokedfactory agent qa3 times,factory eval3 times,factory agent evaluator0 timesTest plan
grep -r "factory agent evaluator" skills/returns no resultsgrep -r "factory agent reviewer" skills/returns no resultsgrep -r "archivist" skills/workflow-discover/SKILL.mdreturns the new archival stepgrep -r "archivist" skills/workflow-review/SKILL.mdreturns the new archival step🤖 Generated with Claude Code