Skip to content

fix: drop redundant tag-row text from /memory dashboard#352

Merged
dcellison merged 1 commit intomainfrom
fix/memory-dashboard-redundancy-351
Apr 20, 2026
Merged

fix: drop redundant tag-row text from /memory dashboard#352
dcellison merged 1 commit intomainfrom
fix/memory-dashboard-redundancy-351

Conversation

@dcellison
Copy link
Copy Markdown
Owner

Closes #351.

Summary

The /memory dashboard renders each tag's count in two places: a text bar-chart block and the inline keyboard buttons below. The text rows duplicate what the buttons already show, so the dashboard is roughly twice as tall as needed. Operator feedback on real data confirmed the bars added no value the button labels weren't already carrying through their (N) suffixes.

Changes

src/kai/memory_command.py:

  • _build_dashboard no longer emits the per-tag rows. The text now stops at the summary header (Memories: N facts across M tags.) and the confidence footer (Tap a tag to browse. Confidence: median X, min Y.).
  • _bar, _BAR_CHAR, _BAR_WIDTH removed - unused after the rows are gone.
  • _build_stats is unchanged; it has its own per-tag rendering that never called _bar.

tests/test_memory_command.py:

  • TestBar (3 tests) deleted along with _bar.
  • test_renders_summary_and_tags reworked: asserts the bar character (▓) is gone, the per-tag row prefixes are gone, and the buttons carry the counts in descending order (preference (5), fact (3), decision (2)).

Net diff: +23 / -56 (-33 lines).

Before / after

Before:

Memories: 73 facts across 8 tags.

  project            18   ▓▓▓▓▓▓▓▓
  fact               16   ▓▓▓▓▓▓▓
  preference         15   ▓▓▓▓▓▓▓
  constraint         10   ▓▓▓▓
  decision            9   ▓▓▓▓
  location            3   ▓
  confirmed_action    1   ▓
  relationship        1   ▓

Tap a tag to browse. Confidence: median 0.86, min 0.52.
[project (18)] [fact (16)] ...
[Search] [Stats]

After:

Memories: 73 facts across 8 tags.

Tap a tag to browse. Confidence: median 0.86, min 0.52.
[project (18)] [fact (16)] ...
[Search] [Stats]

Spec drift

This intentionally diverges from spec 310 §6.1's bar-chart mock-up. All other §6.1 elements (summary header, confidence footer, asymmetry between dashboard and stats around zero-count tags) are preserved. Issue #351 documents the rationale.

Test plan

  • make check passes (ruff check + ruff format).
  • pytest tests/test_memory_command.py - 83 passed.
  • After deploy, send /memory and confirm the dashboard renders without the per-tag rows but with summary + footer + tag buttons + Search/Stats.
  • Confirm /memory stats still renders its full per-tag table (not affected by this change).

Related

The dashboard rendered each tag's count in two places: the text bar-chart
block and the inline buttons immediately below. Operator feedback on real
data (#351) confirmed the bars duplicated what the buttons telegraph
through the parenthesized counts on each label, doubling the dashboard
height for no information gain.

Drop the per-tag text rows. Keep the summary header, the confidence
footer, and the inline keyboard exactly as before. The dashboard text now
stops at the summary + footer, with all per-tag data delegated to the
buttons (which is the intended tap target anyway).

Cleanup follows: _bar(), _BAR_CHAR, and _BAR_WIDTH are removed since
nothing else uses them. _build_stats() has its own per-tag rendering
that does not call _bar(), so the stats screen is unchanged. Tests for
the bar function are removed; the dashboard test is reworked to assert
that tag counts live on the buttons (not in the text) and that the bar
character is gone, guarding the regression.

This intentionally diverges from spec 310 §6.1's bar-chart mock-up. All
other §6.1 elements (summary header, confidence footer, dashboard /
stats asymmetry around zero-count tags) are preserved.
@dcellison
Copy link
Copy Markdown
Owner Author

Review by Kai

The PR looks clean. Here is what I checked:

Logic (no issues)

The removal of max_count = nonzero[0][1] and label_width = ... is safe - both variables were only used by the deleted loop. The early-return guard for nonzero being empty was already in place before those lines, so no new IndexError risk is introduced.

The blank line between summary and footer is preserved: the lines.append("") that follows lines.append(summary) is context, not a deletion. Output matches the "after" example in the description.

Security (no issues)

Purely a UI text change. No user input processing, auth, or data exposure changes.

Tests (well-done)

The comment in the updated test explaining why the assertion uses " {tag}" prefix instead of bare tag names is exactly the right kind of "non-obvious why" comment. The tag_button_labels assertion gives clear ordering coverage.

Style (no issues)

The in-code comment block explaining the spec drift from §6.1 is long but justified - intentional spec divergence with a documented rationale is exactly the scenario that warrants the explanation.

One minor note (suggestion)

The comment added to _build_dashboard mentions issue #351 and spec §6.1 twice (once at the comment, once again in the PR description). The comment itself is slightly redundant ("Per #351... operator feedback on real data showed the bars duplicated what the buttons already telegraph through the parenthesized counts"). Could trim to the core fact, but it is not wrong, and the project style favors thorough comments for non-obvious decisions.

No blockers. Approve.

@dcellison dcellison merged commit 0de70b3 into main Apr 20, 2026
1 check passed
@dcellison dcellison deleted the fix/memory-dashboard-redundancy-351 branch April 20, 2026 17:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Drop redundant tag-row text from /memory dashboard

2 participants