Commit
·
bfc21c1
1
Parent(s):
7d08d65
docs: add decision record for PR #55 evaluation
Browse files
docs/decisions/2025-11-27-pr55-evaluation.md
ADDED
|
@@ -0,0 +1,49 @@
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| 1 |
+
# Decision Record: PR #55 Evaluation
|
| 2 |
+
|
| 3 |
+
**Date**: 2025-11-27
|
| 4 |
+
**PR**: [#55 - adds the initial iterative and deep research workflows](https://github.com/The-Obstacle-Is-The-Way/DeepCritical-HFSpace/pull/55)
|
| 5 |
+
**Author**: @Josephrp
|
| 6 |
+
**Status**: Not merged
|
| 7 |
+
|
| 8 |
+
## Summary
|
| 9 |
+
|
| 10 |
+
PR #55 proposed 17,779 additions and 3,440 deletions across 68 files. After objective third-party review by CodeRabbit, the PR was found to have significant quality issues that block the test suite from running.
|
| 11 |
+
|
| 12 |
+
## CodeRabbit Findings
|
| 13 |
+
|
| 14 |
+
CodeRabbit's automated review identified **35+ critical issues**:
|
| 15 |
+
|
| 16 |
+
| Issue | Count | Severity |
|
| 17 |
+
|-------|-------|----------|
|
| 18 |
+
| Import errors (`AgentResult` doesn't exist in pydantic-ai) | 3 files | Critical - blocks pytest |
|
| 19 |
+
| Missing parentheses on method calls | 26 places | Critical |
|
| 20 |
+
| Tests calling non-existent methods (`validate()` vs `validate_structure()`) | 3 places | Critical |
|
| 21 |
+
| Wrong node ID assertions | 1 place | Critical |
|
| 22 |
+
| Broken pytest fixtures (`return` vs `yield`) | 2 places | Critical |
|
| 23 |
+
|
| 24 |
+
The 3 import errors cause pytest to crash during collection, preventing any tests from running.
|
| 25 |
+
|
| 26 |
+
## Claims vs Reality
|
| 27 |
+
|
| 28 |
+
| Claim | Reality |
|
| 29 |
+
|-------|---------|
|
| 30 |
+
| "nothing is replaced, just added" | `src/orchestrator.py` renamed to `src/legacy_orchestrator.py`; `CLAUDE.md`, `AGENTS.md`, `GEMINI.md` deleted |
|
| 31 |
+
| "3 failing tests on a 13k LoC PR is not a major issue" | Those 3 tests crash pytest during collection - entire test suite cannot run |
|
| 32 |
+
|
| 33 |
+
## Decision
|
| 34 |
+
|
| 35 |
+
The PR was not merged for the following reasons:
|
| 36 |
+
|
| 37 |
+
1. **Code was never executed before submission** - Basic import errors indicate no local testing
|
| 38 |
+
2. **Parallel architecture, not incremental improvement** - Introduces entirely different orchestration system rather than building on existing working code
|
| 39 |
+
3. **Maintenance burden** - Would require maintaining two separate orchestration systems
|
| 40 |
+
4. **Existing code labeled "legacy"** - Working, tested code renamed to "legacy" in favor of untested code
|
| 41 |
+
|
| 42 |
+
## Context
|
| 43 |
+
|
| 44 |
+
This project is a HuggingFace Spaces hackathon entry. All contributors have direct push access to the HuggingFace Space. Contributors are encouraged to push directly to production when confident in their code, rather than submitting PRs with untested code for others to review and take responsibility for.
|
| 45 |
+
|
| 46 |
+
## Links
|
| 47 |
+
|
| 48 |
+
- [PR #55](https://github.com/The-Obstacle-Is-The-Way/DeepCritical-HFSpace/pull/55)
|
| 49 |
+
- [CodeRabbit Review](https://github.com/The-Obstacle-Is-The-Way/DeepCritical-HFSpace/pull/55#issuecomment-3587631560)
|