From 759298abcb89f252639973c20e190639bcde11bd Mon Sep 17 00:00:00 2001 From: Aakash Kattelu Date: Mon, 10 Aug 2026 17:22:39 -0400 Subject: [PATCH] fix(cli): carry invocation scope into the next-page hint MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 --- honcho-cli/src/honcho_cli/commands/message.py | 3 ++ honcho-cli/src/honcho_cli/commands/session.py | 42 ++++++++++++++++++- honcho-cli/src/honcho_cli/output.py | 16 +++---- honcho-cli/tests/test_commands.py | 37 ++++++++++++++++ honcho-cli/tests/test_output.py | 10 ++--- 5 files changed, 90 insertions(+), 18 deletions(-) diff --git a/honcho-cli/src/honcho_cli/commands/message.py b/honcho-cli/src/honcho_cli/commands/message.py index 40aa83af..7a773fcc 100644 --- a/honcho-cli/src/honcho_cli/commands/message.py +++ b/honcho-cli/src/honcho_cli/commands/message.py @@ -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) diff --git a/honcho-cli/src/honcho_cli/commands/session.py b/honcho-cli/src/honcho_cli/commands/session.py index 5f63f87c..22f4a371 100644 --- a/honcho-cli/src/honcho_cli/commands/session.py +++ b/honcho-cli/src/honcho_cli/commands/session.py @@ -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, ) diff --git a/honcho-cli/src/honcho_cli/output.py b/honcho-cli/src/honcho_cli/output.py index 39f9a2aa..2f0ec5e9 100644 --- a/honcho-cli/src/honcho_cli/output.py +++ b/honcho-cli/src/honcho_cli/output.py @@ -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}") diff --git a/honcho-cli/tests/test_commands.py b/honcho-cli/tests/test_commands.py index 79b387b9..a24d6c77 100644 --- a/honcho-cli/tests/test_commands.py +++ b/honcho-cli/tests/test_commands.py @@ -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"})) diff --git a/honcho-cli/tests/test_output.py b/honcho-cli/tests/test_output.py index b136d99c..50352d04 100644 --- a/honcho-cli/tests/test_output.py +++ b/honcho-cli/tests/test_output.py @@ -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 == []