Repository navigation
Conversation
331693b to
14ff34d
Compare
|
Review the following changes in direct dependencies. Learn more about Socket for GitHub.
|
Keep historian observations in bounded session memory and use indexed cache-delta counts. Share the refresh scheduler across OpenCode sidebar paths without changing their layout.
Move the sidebar frame from @cortexkit/common-auth/quota-sidebar to @cortexkit/opencode-sidebar 0.2.0. Use its SidebarGaugeRow for the context gauge in place of the local gauge, and its collapsible prop to collapse the panel to one row for each section (Context, Historian, Cache, and Health only when something is wrong). The render snapshots store each row as an exact string, so trailing cells are compared and no line ends in whitespace.
14ff34d to
0f27af9
Compare
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
2 issues found across 41 files
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. When an issue isn't valid or won't be fixed in this PR, reply in its thread with the reason and then resolve the thread. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="packages/plugin/src/tui/data/context-db.test.ts">
<violation number="1" location="packages/plugin/src/tui/data/context-db.test.ts:94">
P2: This test does not exercise scheduled reads; every call supplies `{ throwOnFailure: true }` directly. It would pass if the sidebar callers stopped forwarding the option, so test the scheduler’s `read` callback or rename this as a `loadSidebarSnapshot` option test.</violation>
</file>
<file name="packages/plugin/src/hooks/magic-context/compartment-trigger.ts">
<violation number="1" location="packages/plugin/src/hooks/magic-context/compartment-trigger.ts:402">
P2: `waitingMessages` is updated only after the authoritative tail inspection, but the cheap pre-gate can return before reaching this code. After compaction, short or below-floor tails can leave the sidebar showing the old positive count; update this observation on the cheap-skip and other early-return paths as well.</violation>
</file>
Reply with feedback, questions, or to request a fix.
View guided diff | Re-trigger cubic
| } | ||
|
|
||
| describe("TUI context RPC data", () => { | ||
| test("scheduled reads expose RPC failures instead of hiding them behind a cached snapshot", async () => { |
There was a problem hiding this comment.
P2: This test does not exercise scheduled reads; every call supplies { throwOnFailure: true } directly. It would pass if the sidebar callers stopped forwarding the option, so test the scheduler’s read callback or rename this as a loadSidebarSnapshot option test.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. When an issue isn't valid or won't be fixed in this PR, reply in its thread with the reason and then resolve the thread. At packages/plugin/src/tui/data/context-db.test.ts, line 94:
<comment>This test does not exercise scheduled reads; every call supplies `{ throwOnFailure: true }` directly. It would pass if the sidebar callers stopped forwarding the option, so test the scheduler’s `read` callback or rename this as a `loadSidebarSnapshot` option test.</comment>
<file context>
@@ -91,6 +91,25 @@ async function startServer(
}
describe("TUI context RPC data", () => {
+ test("scheduled reads expose RPC failures instead of hiding them behind a cached snapshot", async () => {
+ const dataHome = makeDataHome();
+ const directory = "/repo-scheduled";
</file context>
There was a problem hiding this comment.
Fixed in 86c8aec. The test is renamed to describe the loadSidebarSnapshot option it exercises; it no longer claims to cover scheduler forwarding.
|
|
||
| const rawEligibility = getRawHistoryEligibility(db, sessionId); | ||
| if (!rawEligibility.hasRawBeyondLastCompartment) { | ||
| rememberWaitingMessages(sessionId, 0); |
There was a problem hiding this comment.
P2: waitingMessages is updated only after the authoritative tail inspection, but the cheap pre-gate can return before reaching this code. After compaction, short or below-floor tails can leave the sidebar showing the old positive count; update this observation on the cheap-skip and other early-return paths as well.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. When an issue isn't valid or won't be fixed in this PR, reply in its thread with the reason and then resolve the thread. At packages/plugin/src/hooks/magic-context/compartment-trigger.ts, line 402:
<comment>`waitingMessages` is updated only after the authoritative tail inspection, but the cheap pre-gate can return before reaching this code. After compaction, short or below-floor tails can leave the sidebar showing the old positive count; update this observation on the cheap-skip and other early-return paths as well.</comment>
<file context>
@@ -398,6 +399,7 @@ function getUnsummarizedTailInfo(
const rawEligibility = getRawHistoryEligibility(db, sessionId);
if (!rawEligibility.hasRawBeyondLastCompartment) {
+ rememberWaitingMessages(sessionId, 0);
return { ...TAIL_INFO_DEFAULTS, nextStartOrdinal: rawEligibility.offset };
}
</file context>
There was a problem hiding this comment.
Fixed in 86c8aec. The cheap gate returns only when usage is below the proactive floor and the tail's token upper bound is below the trigger budget, so it cannot know the exact eligible-message count. That path (and the spent-drain and inspection-failure paths) now invalidates the observation, and the sidebar shows no reading instead of a stale count or an invented zero. Counts from an inspected tail are unchanged. Regression: compartment-trigger.test.ts "invalidates a stale waiting count when a below-floor small tail cheap-skips".
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| return readNewMemoriesForM1Union( | ||
| db, | ||
| projectPaths, | ||
| afterId, | ||
| expiryCutoff, | ||
| ownIdentities, | ||
| shareCategories, | ||
| ).length; |
There was a problem hiding this comment.
countNewMemoriesForM1Union now loads every matching memory, including its full text, sorts the rows, and creates memory objects just to count them. Each sidebar refresh repeats this work. When a session has no cache watermark, it reads all visible project and workspace memories.
This adds avoidable database reads and memory use as projects grow. Keep the count in SQL while preserving the reader's validity and visibility checks.
Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
There was a problem hiding this comment.
All reported issues were addressed across 20 files (changes from recent commits).
Reply with feedback, questions, or to request a fix.
View guided diff | Re-trigger cubic
In Rust transform mode the queued-drop count comes from the module's own drop queue, but the token estimate came from the TypeScript pending_ops table, which describes a different queue. The snapshot now omits the estimate when the module supplies the count, and the sidebar shows the count with "no reading" in place of the estimate.
Approved issue: #839
Summary
SidebarFramefrom@cortexkit/opencode-sidebar0.2.0 and shows option B from Sidebar: adopt the shared CortexKit sidebar layout (design proposal, PR offered) #839: Context, History, Prompt cache, Knowledge, Dreamer, and aHealthsection that shows only when something is wrong. The panel collapses to one row per section.collapsibleprop for a panel without accounts, and the exportedSidebarGaugeRow. The local gauge is gone.Screenshots
Rendered from the compiled sidebar slot with OpenTUI's
testRender, with the OpenCode dark theme colours and fixture values (the values of the #839 mocks).Healthin the warning tone)Changes
Snapshot data
All new
SidebarSnapshotfields are optional, so an older RPC server still works with the new view.historianRunwaitingMessagesproactiveFloorgetProactiveCompartmentTriggerPercentage(executeThreshold)queuedDropTokensidx_pending_ops_session_tag_idand thetagsprimary key.cachedM0MaterializedAt,summariesSinceCache,memoriesSinceCachesession_meta, then two counts. The plans usesqlite_autoindex_compartments_1andidx_memories_project_status_expires. The memory count uses the same workspace visibility rules asm[1].configReloadFailureA historian attempt counts against the model that produced it. A primary attempt counts as rejected when its output fails validation, or when the host answers with a different model. An invalid fallback output does not count against the primary. The run's primary is the model it started with, after master's start selection (
selectHiddenModelStart).OpenCode 1 and OpenCode 2 use the same snapshot builder. In Rust transform mode,
historianRunandwaitingMessagesare missing, because the TypeScript historian does not run.Refresh
loadSidebarSnapshothas a new opt-inthrowOnFailure, so that scheduled reads can see a failure. Other callers do not change.View
▼/▶). Collapsed, the panel shows one row for each section in words: Context (45% left), Historian (idle · 1h55m), Cache (151 drops queued), andHealth(2 issues, in the warning or error tone) only when something is wrong. The collapse preference that MC already stores is now used.Healthwording follows the shared CortexKit sidebar standard: stat rows in the warning or error tone, no glyphs, and at most one muted hint row,/ctx-status shows details.SidebarGaugeRow: the warning tone at the session's execute threshold and the error tone at 85%.—. An unknown usage showsno reading, not 100%.Hygiene, the dreamer backlog row for each task, the second dreamer progress line, andStats · Total tokens.historianto History,statusto Prompt cache and the notes rows,memoryto Memories,dreamerto Dreamer. The preferences file format does not change.Healthrows are not shown (collapsed too).Build (decision 3)
scripts/build-tui.tsbundles@cortexkit/opencode-sidebar(it ships TSX source) with the Solid transform bound to the host runtime ids, plus a raw variant. This is the same method that anthropic-auth uses. The script change is +69/−5 lines, and the two bundles are 28,792 B (raw) and 29,058 B (host).bun.lockchanges for the dependency swap.bun installon Bun 1.4.4 also wrote the current workspace versions (0.49.35 to 0.49.52) into it.@cortexkit/opencode-sidebar0.2.0 declares OpenTUI>=0.4.5, so the plugin now declares@opentui/coreand@opentui/solidpeers>=0.4.5and development dependencies^0.4.5(lock: OpenTUI 0.4.2 to 0.4.5 andbun-ffi-structs0.2.3 to 0.2.4). A test fails when the plugin's floor is below the shared sidebar's floor.bun-version: latest, which is 1.4.4 now, socheck:tui-compiledpasses. A later Bun release can change the output for each contributor. A pinned Bun version in CI would prevent this.Not in this PR
Tests
86c8aec, one commit for the 12 review findings): each code fix has a test that failed before it and passes after it (snapshot counts and the tag-number join, the stale waiting count, the native-window gauge, the drop-only queued count, the cumulative fallback row, stopped and failed rebuilds in Health, stale.jscleanup, the fixture clock and the preference writer in the render script).86c8aec:typecheckpasses, Biome passes on the changed files,build:tuitwice writes no file,check:tui-compiledpasses, and the focused TUI, sidebar RPC, compartment-trigger and memory-storage tests pass (188 of 188).86c8aecover masterdba0c4f4e(bun test --parallel=2, otherwise the package'stestcommand): 10,756 pass, 11 fail, 106 skip across 1,084 files. None of the 11 comes from this branch:npm pack --json, andscripts/tui-pack-graph.test.tsand the 4 packed review-bundle tests read[0].files. They fail the same way without this branch.apply-operations.test.ts(1) andscripts/calibrate-tokenizer/credentials-cli.test.ts(5, new on master) spawnprocess.execPath. My Bun 1.4.4 runner is OpenCode's embedded Bun, soprocess.execPathis not a plainbun. Both files pass on Bun 1.4.0 (10 of 10) and this branch does not change them.Harness coverage (required)
For each entry select: covered / not applicable — surface does not exist there / NOT covered.
The PR appears safe to merge, with the earlier non-blocking memory-count cost still present.Findings
Summary
Moves the sidebar onto the shared CortexKit frame and adds optional readings for history, cache, and health.
Reviews (3) · Last reviewed commit: "fix(sidebar): leave the drop token estim..." · Reviewed by Greptile