|
| 1 | +# Review 671: Hermes Consult Backend (Optional, Not Default) |
| 2 | + |
| 3 | +## Summary |
| 4 | +Implemented Hermes as an additional consult backend and kept default consultation fan-out unchanged. Added large-prompt protection for Hermes to avoid CLI argument length failures. Updated docs and tests accordingly. |
| 5 | + |
| 6 | +## Spec Compliance |
| 7 | +- [x] Hermes backend available via `consult -m hermes`. |
| 8 | +- [x] Porch validation accepts `hermes`. |
| 9 | +- [x] Default consultation models remain `gemini`, `codex`, `claude`. |
| 10 | +- [x] Large Hermes prompts use temp-file indirection (ARG_MAX/E2BIG mitigation). |
| 11 | +- [x] Source/skeleton docs synchronized and clarified as optional Hermes. |
| 12 | +- [x] Tests updated for acceptance and large-prompt behavior. |
| 13 | + |
| 14 | +## Implementation Notes |
| 15 | +Key changes landed in: |
| 16 | +- `packages/codev/src/commands/consult/index.ts` |
| 17 | +- `packages/codev/src/commands/porch/next.ts` |
| 18 | +- `packages/codev/src/cli.ts` |
| 19 | +- `codev/resources/commands/consult.md` |
| 20 | +- `codev-skeleton/resources/commands/consult.md` |
| 21 | +- `.gitignore` |
| 22 | +- test files under `packages/codev/src/__tests__/...` |
| 23 | + |
| 24 | +## Validation |
| 25 | +Executed targeted build/test flow from `packages/codev/`: |
| 26 | +- `pnpm build` |
| 27 | +- `pnpm exec vitest run src/commands/porch/__tests__/consultation-models.test.ts src/commands/consult/__tests__/persistent-output.test.ts src/__tests__/consult.test.ts` |
| 28 | +- `pnpm exec vitest run --config vitest.cli.config.ts src/__tests__/cli/consult.e2e.test.ts` |
| 29 | + |
| 30 | +All passed at execution time. |
| 31 | + |
| 32 | +## Deviations and Corrections |
| 33 | +- Initial docs examples incorrectly implied 4-way default review. |
| 34 | +- Corrected docs to explicitly show 3-way default and Hermes as optional. |
| 35 | +- Initial implementation drifted from the spec by treating Hermes as a default consultation backend. Review feedback corrected this before merge by restoring 3-way defaults and moving Hermes coverage to explicit opt-in paths. |
| 36 | + |
| 37 | +## Consultation Feedback |
| 38 | + |
| 39 | +### Specify Phase (Round 1) |
| 40 | + |
| 41 | +#### Gemini |
| 42 | +- No concerns raised — approved. |
| 43 | + |
| 44 | +#### Codex |
| 45 | +- No concerns raised — approved. |
| 46 | + |
| 47 | +#### Claude |
| 48 | +- No concerns raised — approved. |
| 49 | + |
| 50 | +#### Hermes |
| 51 | +- No concerns raised — approved. |
| 52 | + |
| 53 | +### Plan Phase (Round 1) |
| 54 | + |
| 55 | +#### Gemini |
| 56 | +- No concerns raised — approved. |
| 57 | + |
| 58 | +#### Codex |
| 59 | +- No concerns raised — approved. |
| 60 | + |
| 61 | +#### Claude |
| 62 | +- No concerns raised — approved. |
| 63 | + |
| 64 | +#### Hermes |
| 65 | +- No concerns raised — approved. |
| 66 | + |
| 67 | +### Implement Phase 1 (Round 1) |
| 68 | + |
| 69 | +#### Gemini |
| 70 | +- No concerns raised — approved. |
| 71 | + |
| 72 | +#### Codex |
| 73 | +- No concerns raised — approved. |
| 74 | + |
| 75 | +#### Claude |
| 76 | +- No concerns raised — approved. |
| 77 | + |
| 78 | +#### Hermes |
| 79 | +- No concerns raised — approved. |
| 80 | + |
| 81 | +### Implement Phase 2 (Round 1) |
| 82 | + |
| 83 | +#### Gemini |
| 84 | +- **Concern**: Hermes must remain optional rather than becoming part of the default fan-out. |
| 85 | + - **Addressed**: Restored the default consultation models to `gemini`, `codex`, `claude` and reverted default-facing fixtures to 3-way behavior. |
| 86 | + |
| 87 | +#### Codex |
| 88 | +- **Concern**: `afx bench` must not require Hermes by default. |
| 89 | + - **Addressed**: Reverted `ENGINES` to the 3-engine default set and updated the bench tests accordingly. |
| 90 | + |
| 91 | +#### Claude |
| 92 | +- **Concern**: Tests should distinguish between allowed backends and default backends. |
| 93 | + - **Addressed**: Kept Hermes in `VALID_MODELS`, restored 3-way default assertions, and added explicit opt-in Hermes test coverage. |
| 94 | + |
| 95 | +## Architecture Updates |
| 96 | +No architecture updates needed. The final change preserves the existing consultation architecture: Hermes remains an allowed backend, but the default orchestration and benchmark topology stay unchanged. |
| 97 | + |
| 98 | +## Lessons Learned Updates |
| 99 | +No lessons learned updates needed. This correction reinforces existing lessons already recorded in `codev/resources/lessons-learned.md` about preserving defaults unless product intent explicitly changes and keeping allowlists distinct from defaults. |
| 100 | + |
| 101 | +## Flaky Tests |
| 102 | +- No flaky tests encountered in the scoped validation flow used for this project. |
| 103 | + |
| 104 | +## Pull Request |
| 105 | +- Existing PR used for this work: `#670` |
| 106 | + |
| 107 | +## Follow-ups |
| 108 | +- If additional CLI backends are added later, keep default fan-out stable unless explicitly changed by product decision. |
| 109 | +- Preserve large-prompt transport tests for each CLI backend to prevent ARG_MAX regressions. |
0 commit comments