Skip to content

feat(builder): render-mode-aware tool output (plain text in TUI, JSON in CLI) - #109

Merged
iap merged 1 commit into
mainfrom
feat/render-mode-output
Sep 13, 2026
Merged

iap merged 1 commit into
mainfrom
feat/render-mode-output

Conversation

@iap

@iap iap commented Sep 13, 2026

Copy link
Copy Markdown
Owner

TUI mode: returns plain text for human readability (no JSON walls of text).
CLI/standalone: returns JSON for scriptability.

Tools affected: models, tags, q_debug.
All 241 tests pass.

Uses existing _format.load_render_prefs() to detect render_mode.

RetriggerConfidence Score: 5/5

Safe to merge.

Summary

  • This change keeps registered tool handlers returning structured JSON while applying human-readable formatting only for explicit TUI display. The debug view also shows an explicit negative refresh-token state.

Reviews (3) · Last reviewed commit: "feat(builder): render-mode-aware tool ou..."

@github-actions github-actions Bot added the enhancement New feature or request label Sep 13, 2026
Comment thread _format.py Outdated
Comment thread _format.py Outdated
@iap
iap force-pushed the feat/render-mode-output branch from 932f087 to a71ed8c Compare September 13, 2026 13:45

@iap iap left a comment

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review (verified on macOS, PR head a71ed8c) — do not merge as-is

P1: keep handlers JSON, move presentation out (confirmed valid)

I independently verified Greptile's "Preserve tool result structure" finding three ways — it is not a false positive:

  1. verify.py:184,195 does json.loads() on handler outputs. With render_mode: tui in a real Hermes config, models/tags/q_debug return plain text and the gate crashes instead of gating.
  2. CI green is camouflage, not safety. In CI/standalone hermes_cli isn't importable, load_render_prefs() falls back to auto, and every test exercises only the JSON path. "241 pass" proves nothing about the 108 changed lines.
  3. Contract violation. AGENTS.md + _tool_result_helpers require handler output byte-identical to core tool JSON. A display preference must never change the tool-result envelope.

Suggested rework (leaving the design choice to the feature owner):

  1. Handlers return JSON always — restore _success(...) routing for models/tags/q_debug.
  2. Presentation moves outside the tool boundary (separate non-tool helper, TUI-side rendering, or dropped). Delete the dead bid_status branch in format_output either way — no handler routes through it.
  3. Add TUI-path tests for whatever presentation survives (currently zero coverage on the new branches; existing tests only pin CLI mode).
  4. P2 one-liner (valid, trivial): if identity.get("has_refresh_token"): conflates missing with False — check is not None before rendering yes/no.
  5. Retitle scope-less (feat: …) per the current convention.

Verification gate I will run on the next push: verify.py under stubbed render_mode: tui (today's breaking case), full suite, Greptile re-review.

Checked and cleared

  • token_expires_at_iso: real key from sso_oidc (line 547, None default at 596) — the payload addition is sound.
  • Suite green locally (241 passed) — expected per above, not evidence of correctness.

Co-author: Hermes 6572003+iap@users.noreply.github.com

@iap
iap force-pushed the feat/render-mode-output branch from a71ed8c to 076ab76 Compare September 13, 2026 15:14
@iap
iap force-pushed the feat/render-mode-output branch from 076ab76 to 6bac0e4 Compare September 13, 2026 15:16
@iap
iap merged commit 0d8ff2a into main Sep 13, 2026
8 checks passed
@iap
iap deleted the feat/render-mode-output branch September 13, 2026 15:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant