fix(mcp): cap recent_activity rows with explicit truncation footer#785
fix(mcp): cap recent_activity rows with explicit truncation footer#785phernandez merged 2 commits intomainfrom
Conversation
The project-mode formatter previously hardcoded a display cap of 5 entities and 5 relations, but the heading and the "Activity Summary" line both reported the true count returned by the API. The result was a misleading silent truncation: e.g. a heading of "Recent Notes & Documents (9)" with only 5 rows below it and no signal that 4 had been dropped. This raises the cap to 10 (matching the default page_size) and, when the cap is hit, appends an explicit "…and N more on this page (raise page_size to display all)" line so the truncation is visible to the caller. Below the cap, behavior is unchanged. Adds two regression tests covering both the below-cap (no footer) and above-cap (footer present, body truncated to cap) paths. Closes #784 Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> Signed-off-by: Drew Cain <groksrc@gmail.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: ef18845c84
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| lines.append(f" • {title}{folder}") | ||
| if len(entities) > _PROJECT_OUTPUT_DISPLAY_CAP: | ||
| hidden = len(entities) - _PROJECT_OUTPUT_DISPLAY_CAP | ||
| lines.append(f" …and {hidden} more on this page (raise page_size to display all)") |
There was a problem hiding this comment.
Fix misleading truncation footer guidance
The new footer text says users can “raise page_size to display all,” but truncation is controlled by _PROJECT_OUTPUT_DISPLAY_CAP, not page_size. In calls like recent_activity(page_size=50), the API can return more than 10 rows while _format_project_output still renders only 10, so increasing page_size will never reveal the hidden rows and the instruction is incorrect. This creates a user-visible dead-end in project-mode output whenever page_size exceeds the display cap.
Useful? React with 👍 / 👎.
|
Claude finished @groksrc's task in 1m 26s —— View job PR Review: fix(mcp): cap recent_activity rows with explicit truncation footer
SummaryThe fix correctly addresses the silent truncation problem from #784: the old code showed 5 rows while the heading claimed the true total, with no indication anything was hidden. The approach — a named constant, explicit footer — is sound. Two issues need attention before merging. Issues🔴 P1 — Truncation footer gives incorrect advice
lines.append(f" …and {hidden} more on this page (raise page_size to display all)")Codex flagged this and it's correct. Two options: Option A — Drop the separate display cap entirely. Since Option B — Keep the cap but fix the footer message. If the cap is intentional (keeping LLM context compact), the message should reflect how to actually get more rows: lines.append(f" ...and {hidden} more on this page (call again with a smaller page_size or filter by type to see remaining items)")Option A is simpler and removes a confusing two-layer pagination system. Option B keeps the readability guardrail but needs an accurate message. Either way, the current text is misleading and should not ship as-is. 🟡 P2 — Unicode
lines.append(f" …and {hidden} more on this page ...")CLAUDE.md explicitly states: "Avoid using emojis and unicode characters in console and log output." The lines.append(f" ...and {hidden} more on this page ...")The test assertions use the same character and need updating too ( 🔵 Minor — Unrelated reformatting in the diff The diff includes two reformats of pre-existing multi-line calls to single lines ( Checklist
The underlying fix is correct and the test structure is solid. Resolve the two issues above and this is good to go. |
Address review feedback on #785. Replaces the 10-row display cap (and its truncation footer) with no cap at all — `page_size` is now the single knob controlling how many rows come back, so the heading count and body row count always agree. This is the #784 fix without the two-layer pagination system the first pass introduced. The rejected truncation footer told callers to "raise page_size to display all", which was misleading: `_PROJECT_OUTPUT_DISPLAY_CAP` was display-only, independent of `page_size`, so raising it didn't help. Dropping the cap removes the entire class of confusion. Also drops the test for the removed cap behavior; widens the renders-all test to 15 entities / 12 relations to comfortably exceed the old hardcoded `[:5]` slice. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> Signed-off-by: Drew Cain <groksrc@gmail.com>
|
Pushed 6b348df addressing the review. P1 (footer message) — went with Option A: dropped P2 (unicode Minor (unrelated reformatting): kept the format-driven changes. They're not unrelated — main isn't currently ruff-formatted, and Test changes: the cap-behavior test was removed (no cap to test). The renders-all test was widened to 15 entities / 12 relations (comfortably above the old hardcoded
|
|
Claude finished @groksrc's task in 1m 45s —— View job PR Review: fix(mcp): cap recent_activity rows with explicit truncation footer (follow-up)
SummaryThe follow-up commit fully addresses both P1 and P2 from the prior review. Option A (drop the cap entirely) was the right call — it eliminates the two-layer pagination system and makes Changes Reviewed
Checklist
All issues from the prior review are resolved. This is good to merge. |
Summary
Fixes #784. The project-mode formatter in
recent_activitypreviously hardcoded a display cap of 5 entities and 5 relations, but the heading and the trailing "Activity Summary" line both reported the true total returned by the API. The result was a misleading silent truncation — e.g. a heading reading "Recent Notes & Documents (9)" with only 5 rows below it and no signal that 4 had been dropped.This PR keeps the cap-as-readability-guardrail intent but makes it explicit:
page_size) via a new module constant_PROJECT_OUTPUT_DISPLAY_CAP.…and N more on this page (raise page_size to display all)line so the truncation is visible to the caller.The same pattern is applied to both entities and relations in
_format_project_output. Observation grouping (which has its own bespoke caps for category/example counts) is intentionally untouched in this PR — it's a separate design call.Test plan
uv run pytest tests/mcp/test_tool_recent_activity.py— 17 passed.test_recent_activity_format_project_output_renders_all_entities_and_relations— below-cap path: every row is rendered, no truncation footer.test_recent_activity_format_project_output_caps_with_explicit_truncation— above-cap path: body truncated to_PROJECT_OUTPUT_DISPLAY_CAP, headings still report true totals, footer present with correct hidden counts.uv run ruff check— clean on changed files.uv run ruff format— applied.uv run ty check src/basic_memory/mcp/tools/recent_activity.py— passes.🤖 Generated with Claude Code