phasespace-labs / phasespace-labs/palinode

cli: palinode blame, rollback and push print a Python dict instead of JSON when piped

Open
#210 2 comments 0 reactions 1 assignee Claimed by @1cbyc View on GitHub
bug good first issue
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

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.