GeezerVisonThe World Wide Web, restored to 800 pixels of glory. | ||||||||||||||||||||||||||||||
| Home|About|Gaming cabinet|Archives|How it works|Starter sites | ||||||||||||||||||||||||||||||
GeezerVison is showing github.com — feat: trigger compaction independently for the chat and compaction models by ibetitsmike · Pull Request #28568 · coder/coder Conversation
|
|
@codex review |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
The reason will be displayed to describe this comment to others. Learn more.
Here are some automated review suggestions for this pull request.
Reviewed commit: 6c9d5e5fb0
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Sorry, something went wrong.
There was an error while loading. Please reload this page.
There was an error while loading. Please reload this page.
There was an error while loading. Please reload this page.
|
@codex review |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
The reason will be displayed to describe this comment to others. Learn more.
Here are some automated review suggestions for this pull request.
Reviewed commit: 4d3b53e70e
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Sorry, something went wrong.
There was an error while loading. Please reload this page.
There was an error while loading. Please reload this page.
There was an error while loading. Please reload this page.
|
@codex review |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
The reason will be displayed to describe this comment to others. Learn more.
Here are some automated review suggestions for this pull request.
Reviewed commit: 7ff36a0e13
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Sorry, something went wrong.
There was an error while loading. Please reload this page.
There was an error while loading. Please reload this page.
There was an error while loading. Please reload this page.
There was an error while loading. Please reload this page.
|
@codex review |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
The reason will be displayed to describe this comment to others. Learn more.
Here are some automated review suggestions for this pull request.
Reviewed commit: 8a34ac1183
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Sorry, something went wrong.
There was an error while loading. Please reload this page.
There was an error while loading. Please reload this page.
There was an error while loading. Please reload this page.
There was an error while loading. Please reload this page.
There was an error while loading. Please reload this page.
There was an error while loading. Please reload this page.
|
@codex review |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
|
Codex Review: Didn't find any major issues. What shall we delve into next? Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
The reason will be displayed to describe this comment to others. Learn more.
Here are some automated review suggestions for this pull request.
Reviewed commit: f5423a3f23
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Sorry, something went wrong.
There was an error while loading. Please reload this page.
There was an error while loading. Please reload this page.
There was an error while loading. Please reload this page.
There was an error while loading. Please reload this page.
Documentation CheckThis PR changes user-facing compaction behavior for the Coder Agents chat feature: compaction now fires at whichever of two independent triggers binds first (chat trigger vs. organization compaction model trigger, lower token point wins), a threshold of 100 disables only its own trigger, and an organization compaction model whose own trigger is disabled (threshold 100) or unreachable (point past the chat window) falls back to the chat model. The published docs do not yet reflect any of these semantics. Updates Needed
Automated review via Coder Agents |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
f5423a3 to
0b6625c
Compare
There was an error while loading. Please reload this page.
There was an error while loading. Please reload this page.
There was an error while loading. Please reload this page.
There was an error while loading. Please reload this page.
There was an error while loading. Please reload this page.
There was an error while loading. Please reload this page.
There was an error while loading. Please reload this page.
0b6625c to
c6054b4
Compare
1a9a4be to
09dc580
Compare
|
@codex review
|
Sorry, something went wrong.
There was an error while loading. Please reload this page.
There was an error while loading. Please reload this page.
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
|
Codex Review: Didn't find any major issues. Keep them coming! Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
The reason will be displayed to describe this comment to others. Learn more.
Backend ✅
Docs have a TODO we shouldn't merge.
Sorry, something went wrong.
There was an error while loading. Please reload this page.
There was an error while loading. Please reload this page.
There was an error while loading. Please reload this page.
There was an error while loading. Please reload this page.
There was an error while loading. Please reload this page.
There was an error while loading. Please reload this page.
There was an error while loading. Please reload this page.
There was an error while loading. Please reload this page.
There was an error while loading. Please reload this page.
There was an error while loading. Please reload this page.
There was an error while loading. Please reload this page.
There was an error while loading. Please reload this page.
09dc580 to
b28362f
Compare
|
@codex review
|
Sorry, something went wrong.
There was an error while loading. Please reload this page.
…verride window The organization override trigger no longer reads the override model's compression threshold. It fires at a fixed 80% of the override model's context window, leaving room for the summary prompt and the summary. An override with a threshold of 100 is no longer treated as absent; only an unknown window falls back to the chat model. The still-over-limit copy names the 80% window instead of an override threshold.
…ride window The organization override trigger now fires at a fixed 80% of the override model's context window, mirroring chatd, and no longer reads the override model's own compression threshold. Removes the notice for an override whose own compaction is off, and updates the settings popover, organization warning, fixtures, and tests to the new trigger.
…table The site binding cases now match TestBindingCompactionTriggerSource row for row, with the organization trigger at the fixed 80% of the override window.
|
@codex review |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
|
/coder-agents-review |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
The reason will be displayed to describe this comment to others. Learn more.
Here are some automated review suggestions for this pull request.
Reviewed commit: 987cefe399
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Sorry, something went wrong.
There was an error while loading. Please reload this page.
There was an error while loading. Please reload this page.
|
@codex review |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
|
Codex Review: Didn't find any major issues. Keep it up! Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
The reason will be displayed to describe this comment to others. Learn more.
This round covers the rebase onto ff3fa22 and five new commits, which make the organization trigger a fixed 80% of the override window. New findings: 2 P3, 1 P4, 2 Nit.
The new rule follows the updated #30203 decision: neither chatd nor the site reads the override model's compression_threshold, the Go and TypeScript binding tables match row for row, and the rebase keeps main's table layout and required props. The comment and description say the remaining 20% holds the summary request, and nothing checks that (inline). The round 18 approval was for c804a46.
🤖 This review was automatically generated with Coder Agents.
Sorry, something went wrong.
There was an error while loading. Please reload this page.
There was an error while loading. Please reload this page.
There was an error while loading. Please reload this page.
There was an error while loading. Please reload this page.
There was an error while loading. Please reload this page.
There was an error while loading. Please reload this page.
With the fixed 80% trigger, an override trigger is disabled only for a zero window, which the context_limit > 0 check rejects, so a set override always becomes the organization trigger and summarizer. The 80% comment now calls the rest of the window headroom rather than a guarantee, and a test chat title matches its subtest.
…erride copy Renames the window constant to match chatd's compactionOverrideWindowPercent, describes the remaining 20% as target headroom, rewords the override alert and popovers so the organization override is the subject, and drops the organization trigger's enabled check since the database rejects non-positive windows.
|
@codex review |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
|
/coder-agents-review |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
|
Codex Review: Didn't find any major issues. 🎉 Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
The reason will be displayed to describe this comment to others. Learn more.
This round covers 6e8efb9 and c6f8750. No new findings, and no findings are open.
Verified fixed: the unreachable override gate is gone in chatd and the site, the test chat title matches its subtest, the TypeScript constant has chatd's name, and the alert and Effective popovers make the organization override the subject. The 80% comments and the description now call the remaining 20% a target margin, and the summary-time fit check stays out of this PR because the same overshoot exists for every trigger on main.
Out of scope (needs a ticket or explicit acceptance by a human):
coderd/x/chatd/chatloop/compaction.go:224: when the estimated summary input already exceeds the summarizer's window, GenerateCompaction sends the request with the cap unchanged, for every trigger and on main too. Inferred from the code, not run: a provider that enforces the window rejects it, and every later turn fails the same way with a provider error until the user starts a new conversation.🤖 This review was automatically generated with Coder Agents.
Sorry, something went wrong.
There was an error while loading. Please reload this page.
|
/coder-agents-review New head d5888ce after round 20's approval of c6f8750 (093076f was docs only). Two frontend fixes from UAT round 13 notes:
|
Sorry, something went wrong.
There was an error while loading. Please reload this page.
|
Codex Review: Didn't find any major issues. Keep them coming! Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
The reason will be displayed to describe this comment to others. Learn more.
This round covers 093076f, 4e172ab, and d5888ce. New findings: 1 P4.
Verified: the architecture doc's compaction bullets now match chatd, which resolves the round 1 finding about the stale stricter-limit rule there; formatContextLimit keeps every label from 1,000 tokens up, and its new test pins each boundary; the personal header now hedges while overrides load or after they fail. The row's 100% tooltip misses the failed-load case (inline). The round 20 approval was for c6f8750.
Out of scope (needs a ticket or explicit acceptance by a human):
site/src/pages/AgentsPage/components/ContextUsageIndicator.tsx:97: formatTokenCountCompact(999_999) returns "1000K", so the context gauge can show the rounding this PR fixed in formatContextLimit.site/src/modules/aiModels/knownModels/catalog.ts:89: formatContextBadge(999_999) returns "1000K context" in the model catalog badge, the same rounding.site/src/modules/aiModels/ModelSelector.tsx:83: the M branch labels 1,000,000 tokens "1M" and 1,048,576 tokens "1.0M", so nearly equal windows get labels in two formats in the model picker.site/src/modules/aiModels/ModelSelector.tsx:73: formatContextLimit also formats compaction points (UserCompactionThresholdSettings.tsx:141), so its name describes only one of its two uses.🤖 This review was automatically generated with Coder Agents.
Sorry, something went wrong.
There was an error while loading. Please reload this page.
There was an error while loading. Please reload this page.
…ization overrides fail to load
There was an error while loading. Please reload this page.
|
/coder-agents-review CRF-167 is fixed in 3cd5b08: the row's 100% tooltip now hedges when its organization's overrides failed to load, with a new story for that state. Thread replied and resolved. Out of scope, left unchanged (pre-existing on main):
|
Sorry, something went wrong.
There was an error while loading. Please reload this page.
|
Codex Review: Didn't find any major issues. What shall we delve into next? Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
The reason will be displayed to describe this comment to others. Learn more.
This round covers 3cd5b08. No new findings, and no findings are open.
Verified fixed: when an organization's overrides fail to load, the row's 100% tooltip now says an organization override may still compact chats with this model, matching the header. A failed load never leaves a trigger for that organization, and rows stay hidden while overrides load, so the tooltip and the header agree in every state the rows render in.
🤖 This review was automatically generated with Coder Agents.
Sorry, something went wrong.
There was an error while loading. Please reload this page.
There was an error while loading. Please reload this page.
There was an error while loading. Please reload this page.
Successfully merging this pull request may close these issues.
There was an error while loading. Please reload this page.