fix(cli): carry invocation scope into the next-page hint

Addresses CodeRabbit review on #1006.

The "more:" hint echoed only `--page`, `--size`, and `--reverse`, so copying
it off a scoped invocation dropped `-w`, `-p`, and `--ids` — landing on a
different workspace or an unfiltered transcript. Hint construction moves into
`_next_page_command` in the command module, which knows the invocation; the
renderer now just prints the string it's handed and no longer needs to know
CLI flag syntax. Only flags passed explicitly are echoed, since anything from
the environment or config resolves the same way on the next run.

Also rejects non-positive `--last` on `honcho message list`, which slice
semantics turned into a silently empty result. `session view` already errored
on it; the two now agree.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This commit is contained in:
Aakash Kattelu 2026-08-10 17:22:39 -04:00
parent 2615351ae6
commit 759298abcb
5 changed files with 90 additions and 18 deletions

View File

@ -37,6 +37,9 @@ def list_messages(
handle_cmd_flags(json_output=json_output, workspace=workspace, peer=peer, session=session)
sid = _get_session_id(session_id)
if last < 1:
print_error("INVALID_FLAGS", "--last must be >= 1", {"last": last})
raise typer.Exit(1)
client, config = get_client()
sess = client.session(sid)

View File

@ -162,6 +162,33 @@ def _fetch_recent_messages(sess, filters: dict | None, last: int) -> tuple[list,
return msgs[:last], total
def _next_page_command(
session_id: str,
next_page: int,
size: int,
*,
reverse: bool,
show_ids: bool,
workspace: str | None,
peer: str | None,
) -> str:
"""Continuation command for the next page, carrying this invocation's scope.
Scoping flags are echoed only when passed explicitly; anything resolved from
the environment or config file resolves the same way on the next run.
"""
parts = [f"honcho session view {session_id}", f"--page {next_page}", f"--size {size}"]
if reverse:
parts.append("--reverse")
if show_ids:
parts.append("--ids")
if workspace:
parts.append(f"-w {workspace}")
if peer:
parts.append(f"-p {peer}")
return " ".join(parts)
def _fetch_all_messages(sess, filters: dict | None) -> tuple[list, int | None]:
"""Fetch every message in the session, oldest first."""
page = sess.messages(filters=filters, reverse=False, size=MAX_PAGE_SIZE)
@ -304,6 +331,18 @@ def view(
_handle_error(e, "session", sid)
raise # unreachable: _handle_error always exits
next_page_hint = None
if page_meta is not None and pages_meta is not None and page_meta < pages_meta:
next_page_hint = _next_page_command(
sid,
page_meta + 1,
page_size,
reverse=reverse,
show_ids=show_ids,
workspace=workspace,
peer=peer,
)
# Rendered outside the try: output failures aren't session API errors.
print_transcript(
items,
@ -311,9 +350,8 @@ def view(
total=total,
page=page_meta,
pages=pages_meta,
size=page_size if mode == "page" else None,
reverse=reverse,
show_ids=show_ids,
next_page_hint=next_page_hint,
)

View File

@ -150,15 +150,14 @@ def print_transcript(
total: int | None = None,
page: int | None = None,
pages: int | None = None,
size: int | None = None,
reverse: bool = False,
show_ids: bool = False,
next_page_hint: str | None = None,
) -> None:
"""Render a session transcript as a row-delimited table, or JSON.
Each message dict must have ``peer_id``, ``content``, ``created_at``;
``id`` is optional and only shown when ``show_ids`` is set. ``size`` and
``reverse`` are echoed back in the next-page hint.
``id`` is optional and only shown when ``show_ids`` is set.
``next_page_hint`` is printed below the table when given.
"""
if use_json():
print_json(messages)
@ -212,10 +211,5 @@ def print_transcript(
table.add_row(*row)
stdout_console.print(table)
if page is not None and pages is not None and page < pages:
hint = f"more: honcho session view {session_id} --page {page + 1}"
if size is not None:
hint += f" --size {size}"
if reverse:
hint += " --reverse"
status(hint)
if next_page_hint:
status(f"more: {next_page_hint}")

View File

@ -15,6 +15,7 @@ from unittest.mock import MagicMock, patch
import pytest
from typer.testing import CliRunner
from honcho_cli.commands.session import _next_page_command
from honcho_cli.main import app
@ -226,6 +227,20 @@ class TestJsonContract:
"created_at": "2026-01-01T00:00:00Z",
}
@pytest.mark.parametrize("last", ["0", "-5"])
def test_message_list_rejects_non_positive_last(self, cfg, runner, last):
"""Non-positive --last silently returned an empty list via slice semantics."""
cfg.write_text(json.dumps({"apiKey": "k", "environmentUrl": "http://localhost:8000"}))
config = MagicMock(session_id="sess1", workspace_id="ws1", peer_id="")
with patch("honcho_cli.commands.message.get_client", return_value=(MagicMock(), config)) as get_client:
result = runner.invoke(
app,
["message", "list", "sess1", "--last", last, "-w", "ws1"],
)
assert result.exit_code == 1
assert json.loads(result.stderr)["error"]["code"] == "INVALID_FLAGS"
get_client.assert_not_called()
def test_session_view_json_is_chronological_window(self, cfg, runner):
"""`session view` returns the most recent N messages oldest→newest by default."""
cfg.write_text(json.dumps({"apiKey": "k", "environmentUrl": "http://localhost:8000"}))
@ -350,6 +365,28 @@ class TestJsonContract:
client.session.assert_not_called()
session_cls.assert_called_once_with("sess1", client)
@pytest.mark.parametrize(
("kwargs", "expected"),
[
(
{},
"honcho session view s1 --page 2 --size 50",
),
(
{"reverse": True, "show_ids": True},
"honcho session view s1 --page 2 --size 50 --reverse --ids",
),
(
{"workspace": "ws2", "peer": "alice"},
"honcho session view s1 --page 2 --size 50 -w ws2 -p alice",
),
],
)
def test_next_page_command_carries_the_invocation_scope(self, kwargs, expected):
"""A copied hint must land on the same workspace, peer, and ordering."""
opts = {"reverse": False, "show_ids": False, "workspace": None, "peer": None, **kwargs}
assert _next_page_command("s1", 2, 50, **opts) == expected
def test_session_view_last_walks_pages_past_the_page_cap(self, cfg, runner):
"""`--last N` above the 100-item server cap keeps walking instead of truncating."""
cfg.write_text(json.dumps({"apiKey": "k", "environmentUrl": "http://localhost:8000"}))

View File

@ -126,14 +126,14 @@ class TestTranscriptFidelity:
class TestNextPageHint:
def test_hint_echoes_size_and_reverse(self, render, capsys, monkeypatch):
def test_given_hint_is_printed(self, render, monkeypatch):
printed: list[str] = []
monkeypatch.setattr(output, "status", printed.append)
render([_msg()], session_id="s1", page=1, pages=3, size=10, reverse=True)
assert printed == ["more: honcho session view s1 --page 2 --size 10 --reverse"]
render([_msg()], session_id="s1", page=1, pages=3, next_page_hint="honcho ... --page 2")
assert printed == ["more: honcho ... --page 2"]
def test_hint_omitted_on_the_last_page(self, render, monkeypatch):
def test_no_hint_when_none_given(self, render, monkeypatch):
printed: list[str] = []
monkeypatch.setattr(output, "status", printed.append)
render([_msg()], session_id="s1", page=3, pages=3, size=10)
render([_msg()], session_id="s1", page=3, pages=3)
assert printed == []