Skip to content

fix failing tests - #9306

Merged
akshaydeo merged 3 commits into
mainfrom
dev
Oct 29, 2026
Merged

akshaydeo merged 3 commits into
mainfrom
dev

Conversation

@akshaydeo

Copy link
Copy Markdown
Contributor

No description provided.

kohlivrinda and others added 3 commits October 30, 2026 00:33
## Overview

`list_topics` answers could grow past `MaxToolResultBytes`. When that happened, the agent loop replaced the answer with "result too large". The model read that as a failed call and retried with a lower limit or switched tools, so one topic question took two or three calls. This PR makes `list_topics` size its own answer so it reaches the model. It also stops declining "how many users asked about this topic?", which `query_usage_by` can already answer.

## Changes

- **Smaller topic rows**
  - Floats are rounded to what an answer would quote: share to 4 places, cost to 6, latency and trends to 1, cohesion to 3.
  - Examples are capped at 3 per topic (`topicsExamplesListed`).
  - `example_log_ids` is no longer sent. The `example_links` entries already carry the same ids, so `SampleLogIDs` is now `json:"-"`.
- **Slimmer topics inside themes**
  - With `group_by theme`, each topic inside a theme is reduced to id, label, requests, share and link, because the theme already carries the counts, trend and series.
  - Each theme gets a `topic_count`, so the true size is still visible when fewer topics are listed.
  - The full topic rows remain available from `list_topics` without `group_by`.
- **Fitting the result to the budget (`fitTopicResult`)**
  - If the encoded answer is still over `MaxToolResultBytes`, the smallest rows are dropped until it fits. The rows are ranked largest first, so this is an honest partial answer.
  - A `trimmed` note says what was cut and tells the model not to page through the rest.
  - Flat lists drop their tail topics.
  - Themed lists first shrink the topics inside every theme one rank at a time, then drop ungrouped topics, then drop the smallest themes.
  - Each pass re-encodes the answer. That costs a tool call, never a gateway request.
- **Counting users behind a topic or theme**
  - The system prompt now tells the model to take the id from `list_topics` and call `query_usage_by` with `dimension user`, `topic_ids` set to that id, the same window and filters, and `limit 20`.
  - It counts rows other than `unassigned` and `other`, and says "at least 20" when 20 rows come back.
  - The model no longer offers the feature-request link for this question.
  - The tool result's `cannot_answer` note is replaced by a `people` note with the same instructions.
- **E2E coverage**
  - The Postman `topics-people` case now expects `list_topics` followed by a `query_usage_by` call with `topic_ids` and `dimension: user`, and a numeric user count in the answer. It also expects no feature-request link, and the Topics folder description is updated to match.
  - The runner that builds the collection (`build-warp-collection.mjs`) has the same changes.

## Type of change

- [x] Bug fix
- [x] Feature
- [ ] Refactor
- [ ] Documentation
- [ ] Chore/CI

## Affected areas

- [x] Core (Go)
- [ ] Transports (HTTP)
- [ ] Providers/Integrations
- [ ] Plugins
- [ ] UI (React)
- [ ] Docs

## How to test

Run the Warp framework tests. `TestWarpListTopicsToolFitsTheToolResultBudget` uses realistic topics (uuid ids, four examples each, long labels, unrounded floats) and checks three cases:

- The default call returns all 15 topics untrimmed, with example links and no `example_log_ids`.
- `limit: 50` keeps the largest topics that fit and reports "N of the 50".
- `group_by: theme` returns every theme with `topic_count`, and a `trimmed` note says the topics inside the themes were cut first.

```sh
# Core/Transports
go version
go test ./framework/warp/...
go test ./...
```

For the e2e case, rebuild the Warp Postman collection with `tests/e2e/api/runners/build-warp-collection.mjs` and run the Topics folder. "How many different users asked about the largest topic?" should call `list_topics` then `query_usage_by` (at most 3 calls) and answer with a user count instead of the feature-request link.

No new configs or environment variables.

## Screenshots/Recordings

Not applicable.

## Breaking changes

- [ ] Yes
- [x] No

The tool-result shape changes for the model only: `example_log_ids` is removed, topics inside themes are slimmer, and `cannot_answer` is replaced by `people`. The log, store and HTTP APIs are unchanged.

## Related issues

None linked.

## Security considerations

None. The change only trims and rounds what `list_topics` returns. Scope and filter handling is unchanged, and the user count goes through `query_usage_by`, which already hides users the caller may not see behind `other`.

## Checklist

- [ ] I read `docs/contributing/README.md` and followed the guidelines
- [x] I added/updated tests where appropriate
- [ ] I updated documentation where needed
- [ ] I verified builds succeed (Go and UI)
- [ ] I verified the CI pipeline passes locally if applicable
@akshaydeo
akshaydeo requested a review from a team as a code owner October 29, 2026 19:41
@akshaydeo
akshaydeo merged commit e869c3f into main Oct 29, 2026
7 of 17 checks passed
@coderabbitai

coderabbitai Bot commented Oct 29, 2026 •

Copy link
Copy Markdown
Contributor

Review in Change Stack →

Caution

Review failed

The pull request is closed.

Finishing Touches
📝 Generate docstrings
  • Commit to this branch
  • Create a new PR
🧪 Generate unit tests (beta)
  • Commit to this branch
  • Create a new PR
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Comment @coderabbitai help to get the list of available commands.

@greptile-apps

greptile-apps Bot commented Oct 29, 2026 •

Copy link
Copy Markdown
Contributor

RetriggerView in GreptileConfidence Score: 5/5

[Medium impact] The PR appears safe to merge; no actionable regressions were found.

Reviews (1) · Last reviewed commit: "fixed pieline for arm build" · Reviewed by Greptile

This branch is waiting to be deployed

1 waiting deployment
pr-testing — c34bd436 Waiting Oct 29, 2026 by akshaydeo via Egress allowlist covers apt + provider-harness hosts #1124
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.

2 participants