fix: keep stdout parseable, correct advertised examples and the auto-JSON claim - #107
Open
GregHolmes wants to merge 4 commits into
Open
fix: keep stdout parseable, correct advertised examples and the auto-JSON claim#107GregHolmes wants to merge 4 commits into
GregHolmes wants to merge 4 commits into
Conversation
Closes #104. Four deepctl_core modules declared bare `Console()` instances bound to stdout and printed diagnostics through them, so `dg -o json <cmd>` on an auth failure wrote 132 bytes of English prose to stdout and left stderr empty -- a JSONDecodeError for the CI step that redirects stdout and parses it, on the single most common way a CI step fails. get_status_console() warns about exactly this pattern in its own docstring. auth, client, timing and base_group_command carry diagnostics only, so their console is now the shared stderr_console. base_command's is deliberately NOT moved: it writes the payload (tables, JSON, raw text from _output_*), which belongs on stdout. It moves from a bare Console() to the shared stdout instance so it stops silently missing the agentic no-color/highlight config -- the second half of the docstring's warning. Moving the prose off stdout left stdout *empty* on failure, which is still unparseable. The guard's `except` now emits an ErrorResult through the normal output path, so stdout carries {"status": "error", "error": ...}. It is a no-op in default mode, so humans see no duplicate of the stderr diagnosis. Verified against the issue's five commands with an invalid key: each now exits 1 with parseable JSON on stdout and the prose on stderr. Tests pin all three properties: which shared console each module holds, that the JSON failure payload parses while default mode stays silent on stdout, and an AST sweep asserting no module in deepctl_core declares a bare Console() at all -- confirmed to fail when one is reintroduced.
Closes #105. `dg usage` advertised `--days 30` and `--start`/`--end`; the real options are `--start-date`/`--end-date` and there is no `--days` at all. The help text contradicted its own options list eight lines further down. The issue suggested sweeping the other commands, which found the same class of error in two more places: - `dg debug stream` -- debug's subcommands are audio, browser, network, probe and toolkit. The WebSocket stream debugging its agent_help describes is `probe` ("Stream probe proxy"). - `dg profiles --show default` -- profiles has --switch, --current and --list; there is no --show. This is worse than a help-text typo because the same `examples` array is what `--agent-friendly` emits, so an agent asking the CLI how to use itself was handed four commands that fail. tests/unit/test_command_examples.py now parses every string in every examples array against the real command tree, one test per example (125 of them). parse_args resolves the command and validates options without invoking the handler, so nothing touches the network. Examples are shell snippets rather than bare argv, so it extracts just the `dg ...` segments -- pipelines, upstream producers and trailing # comments are not ours to validate, and command substitution is skipped outright. `dg debug toolkit`'s subcommands are exempt: they are built from a manifest fetched by `toolkit refresh` and cached on disk, so a clean checkout has only `refresh` and its script examples are unverifiable rather than wrong. A test_examples_were_discovered guard fails if the entry point groups go stale, so the suite cannot silently degrade to testing nothing.
Closes #106. The landing page promised output auto-switches to JSON when stdout is piped. It doesn't: setup_output only flips the format when is_agentic() is true, and is_agentic() needs 3+ soft signals. A plain pipe from an interactive shell scores 1 (stdout not a tty), or 2 if stdin is redirected too -- TERM is set in any normal terminal, so the third point never arrives. Verified directly: with both streams non-tty and TERM=xterm, is_agentic() is False and setup_output("default") leaves the format at "default". Two instances beyond the three the issue names: - README.md said the switch happens in "a non-TTY environment (pipes, CI, or AI coding tools)". CI and AI tools are right; pipes are not. - index.astro:433 is a JSON-LD FAQPage answer -- structured data Google can surface as a rich result, so the false claim travels further than the page. The 'Errors to stderr' bullet is softened rather than kept. #106 offered keeping "Clean stdout channel. No surprises in pipes." if #104 landed first; #104 landed, but the claim is still not true. Probing failure paths across ten commands found `dg ffprobe --path /nonexistent` and `dg debug audio -f /nonexistent.wav` still emitting prose to stdout ahead of the JSON payload, from bare Console() instances in their own packages. Those consoles are mixed -- they also carry the human-readable display that belongs on stdout in default mode -- so routing them needs per-call-site judgment across ~30 command packages rather than a module-level swap. Out of scope here; the bullet now describes the contract without the absolute guarantee. The agent-mode copy at :645 and llms.txt:34 already attributed the switch correctly and are left alone. Verified by building the site: all three visible strings render, the old claims are gone, and the FAQPage schema still parses.
The guard called path.read_text() with no encoding, so it used the locale codec. On Windows that is cp1252, which cannot decode the ⏱️ in timing.py:119, and the test died with UnicodeDecodeError before it could assert anything -- failing every windows-latest job in the matrix while passing on Linux and macOS. Reproduced locally by forcing the codec: read_text(encoding="cp1252") on timing.py raises at byte 3610; utf-8 reads it fine.
GregHolmes
force-pushed
the
fix/output-channel-correctness
branch
from
August 20, 2026 10:51
80c7df5 to
f4bec64
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes the three output-correctness issues filed today. Stacked on #103 — base is
chore/release-hardening, notmain, because the #104 fix builds on theplugin_managerconsole change there. Rebases ontomaincleanly once #103 merges.Closes #104, closes #105, closes #106.
1. Diagnostics off stdout on the failure path (#104)
auth,client,timingandbase_group_commanddeclared bareConsole()instances bound to stdout and printed diagnostics through them, sodg -o json <cmd>on an auth failure wrote 132 bytes of prose to stdout and left stderr empty.get_status_console()warns about exactly this in its own docstring.Those four now use the shared
stderr_console.base_commandis deliberately not moved — its console writes the payload (tables, JSON, raw text from_output_*), which belongs on stdout. It moves from a bareConsole()to the shared stdout instance so it stops silently missing the agentic no-color/highlight config.Moving the prose off stdout left stdout empty, which is still unparseable, so the guard's
exceptnow emits anErrorResultthrough the normal output path. It is a no-op in default mode, so humans see no duplicate of the stderr diagnosis.dg -o json …(invalid key){"status": "error", …}json.load(stdout)2. Advertised examples that don't parse (#105)
dg usageadvertised--days 30and--start/--end; the real options are--start-date/--end-dateand there is no--days. The sweep the issue suggested found the same error twice more:dg debug stream— debug has audio, browser, network, probe, toolkit. The stream debugging itsagent_helpdescribes isprobe.dg profiles --show default— profiles has--switch,--current,--list.Worse than a help-text typo, because the same array is what
--agent-friendlyemits: an agent asking the CLI how to use itself was handed four commands that fail.tests/unit/test_command_examples.pynow parses 125 examples, one test each, viaparse_args— no handler invocation, no network. Examples are shell snippets rather than argv, so it extracts only thedg …segments; pipelines, upstream producers and trailing#comments are not ours to validate, and command substitution is skipped.dg debug toolkit's script subcommands are exempt with a stated reason — they come from a manifest fetched bytoolkit refresh, so a clean checkout has onlyrefresh.3. Auto-JSON attributed to piping (#106)
The landing page promised output auto-switches to JSON when stdout is piped.
setup_outputonly flips the format whenis_agentic()is true, which needs 3+ soft signals; a plain pipe scores 1–2 becauseTERMis set in any normal terminal. Verified directly — with both streams non-tty andTERM=xterm,is_agentic()isFalseandsetup_output("default")leaves the format atdefault.Two instances beyond the three the issue names:
README.md:319said the switch happens in "a non-TTY environment (pipes, CI, or AI coding tools)". CI and AI tools are right; pipes are not.index.astro:433is a JSON-LDFAQPageanswer — structured data Google can surface as a rich result, so the false claim travels further than the page.The agent-mode copy at
:645andllms.txt:34already attributed the switch correctly and are untouched.One deliberate deviation from the issue
#106 offered keeping "Clean stdout channel. No surprises in pipes." if #104 landed first. #104 landed, but the claim is still not true, so the bullet is softened instead. Probing failure paths across ten commands found two more polluters:
Both come from bare
Console()instances in their own command packages. Unlike thedeepctl_coreones, those consoles are mixed — they also carry the human-readable display that legitimately belongs on stdout in default mode — so routing them needs per-call-site judgment across ~30 command packages rather than a module-level swap. Out of scope here; worth a follow-up issue.Tests
test_output_channels.py— which shared console each module holds; the JSON failure payload parses while default mode stays silent on stdout; plus an AST sweep asserting nodeepctl_coremodule declares a bareConsole(). Confirmed to fail when one is reintroduced.test_command_examples.py— 125 parametrised example checks, with atest_examples_were_discoveredguard so the suite cannot silently degrade to testing nothing if the entry point groups go stale.1232 passed, 6 skipped (from 1107). ruff check + format, mypy: clean.
npm run buildinweb/succeeds; the corrected strings render and theFAQPageschema still parses.Note for review
base_command.pyis CRLF in the repo (7 of 235 Python files are, with no.gitattributes). An earlier pass normalised it to LF, which turned a 23-line change into a 1027-line diff; the history was rewritten to preserve CRLF so the diff stays readable. The stray CRLF files are probably worth a separate.gitattributescleanup rather than drive-by normalisation.