Skip to content

fix(sensor): honor CLI display limit - #119

Open
sanjayy0612 wants to merge 2 commits into
uber:mainfrom
sanjayy0612:fix/sensor-display-limit
Open

fix(sensor): honor CLI display limit#119
sanjayy0612 wants to merge 2 commits into
uber:mainfrom
sanjayy0612:fix/sensor-display-limit

Conversation

@sanjayy0612

@sanjayy0612 sanjayy0612 commented Sep 9, 2026

Copy link
Copy Markdown

What type of PR is this? (check all applicable)

  • Refactor
  • Feature
  • Bug Fix
  • Optimization
  • Documentation Update

Related issue: Closes #118

What changed?

  • Pass the parsed --limit value into AgentObserver.display_summary().
  • Print a newest-first, bounded preview using the existing AgentEvent.get_summary() representation.
  • Treat --limit 0 as disabling the preview and reject negative values during argument parsing.
  • Add CLI wiring, validation, ordering, and limit regression tests.

Why?

The CLI advertised --limit as controlling the number of displayed entries, but the value was never consumed. Every value therefore produced the same aggregate-only output.

How did you test it?

  • .venv/bin/pytest -q — 128 passed
  • .venv/bin/ruff check adr_sensor/
  • Ruff checks on the changed test files
  • git diff --check

Potential risks

Low. The default CLI output now includes the two entry summaries the existing default limit promised. API callers can pass limit=0 to retain aggregate-only output. Entry ordering is newest first and normalizes naive timestamps as UTC through the existing timestamp utility.

Copilot AI lite review requested due to automatic review settings September 9, 2026 06:51

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

@CLAassistant

CLAassistant commented Sep 9, 2026

Copy link
Copy Markdown

CLA assistant check
All committers have signed the CLA.

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.

Sensor CLI --limit option has no effect

3 participants