phasespace-labs / phasespace-labs/palinode
cli: palinode blame, rollback and push print a Python dict instead of JSON when piped
- Dominant language
- Python
- Stars
- 39
- Forks
- 42
- Avg merge
- 1d 1h
- Merged PRs (30d)
- 37
Description
`palinode/cli/_format.py` provides `emit_json` and its docstring explains why machine-readable output must never be routed through `rich.Console`. Its closing line is "this makes that the single path."
**Three commands in `palinode/cli/git.py` are not on that path.** Each hands the raw API response dict to `rich.Console` via `console.print(data)`:
- **`blame`** — the branch taken *without* `--claims`
- **`rollback`**
- **`push`**
**What goes wrong.** `console.print()` on a `dict` renders a **Python repr**:
```
{'note': 'contains some text', 'n': None}
```
Single-quoted keys and `None` instead of `null`, so `palinode blame FILE | jq` does not get invalid JSON — it gets output that was never JSON.
**A trap worth naming before you start.** The tempting one-line fix is `console.print(json.dumps(data))`. **Do not do that.** Rich consumes `[...]` as style markup in a *string*, so a value containing `[bold]` or `[git: abc]` comes back with that substring silently deleted — output that parses cleanly and is wrong. (Printing the raw dict does not have this problem, which is why it is not a symptom today and would become one.) Route through `emit_json` / `click.echo` instead.
**`blame --claims` needs the same treatment.** It currently renders prose through `format_claims_resolution` whether or not stdout is a terminal, so it is equally unusable when piped. **Wanted: JSON for both `blame` modes when stdout is not a TTY, and the existing interactive rendering unchanged in both.**
**The correct pattern is in this repository, twice.**
- `palinode/cli/git.py:55` — `history`, the fourth command in the same file, already calls `print_result(data, fmt=get_default_format())`.
- `palinode/cli/depends.py` — the fuller shape: resolve the format once, emit JSON on the JSON path, keep a hand-written human rendering on the other.
**Please do not simply replace `console.print(data)` with `emit_json(data)`.** That would make `palinode blame` unreadable for the human who runs it at a terminal, which is its main use. What is wanted is the TTY-aware split.
**Scope:** `palinode/cli/git.py` and tests. `history` is already correct — leave it alone. No API changes; the response shapes are fine, only the printing is wrong.
**Tests:**
- For `blame` (both with and without `--claims`), `rollback` and `push`: with a non-TTY stdout, the captured output parses with `json.loads`.
- A value containing a `[...]` substring survives that JSON output intact. This is the assertion that pins the trap above shut.
- At least one human-path test that the interactive rendering is unchanged — `blame --claims` is the one that matters, since its prose formatting is deliberate.
**How to see it:** run any of the three with stdout piped and try to parse the result.
Comment here to claim it. **Please wait to be assigned before opening a PR** — two contributors landed on one issue once because we had no such line.
Contributor guide
Assessment
This issue has not been assessed yet.