Conversation
…onfig Replace the separate Swarm, Commits and Compaction margin widgets with one always-visible Overview that folds them in as sections, order matching master: Runtime, Todos, Memory, Background, Usage, KV, Compaction, Changes, Commits, Swarm. Any single section is enough to place it; memory stays in its dedicated MemoryActivity widget, same as master. The runtime panel gains rows for model and agent overrides. Placement fix: mergeable widgets could anchor into the only margin pocket before the Overview had data and squat it indefinitely. Placement now retries once without the mergeable anchors when the Overview is available but not placed, adopting the retry only if the Overview actually lands. MemoryActivity is deliberately not mergeable: the Overview strips memory_info, so suppressing it would hide the only renderer of it. New agents.memory_effort key (env JCODE_MEMORY_EFFORT) pins the reasoning effort of the memory extraction sidecar on every backend: OpenAI reasoning pin, Claude thinking budget or output_config effort derived from provider-core caps, or set_reasoning_effort on the sidecar's independent provider fork. Empty strings trim to unset. The runtime panel shows it next to the memory model row.
 |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
Comment on lines
+197
to
+203
| provider: provider.inspect(|fork| { | ||
| // The fork is independent of the main agent (per the Provider | ||
| // fork contract), so pinning `agents.memory_effort` on it | ||
| // never disturbs the user's live session effort. Providers | ||
| // without an effort knob reject; keep their default. | ||
| if let Some(effort) = configured_effort.as_deref() | ||
| && let Err(err) = fork.set_reasoning_effort(effort) |
There was a problem hiding this comment.
When the active provider is Copilot claude-sonnet-5, its fork shares the live provider’s effort setting. Setting agents.memory_effort here changes the effort used by subsequent main-agent requests instead of isolating it to memory extraction. This must be fixed before merging.
Knowledge Base Used: Model provider integration
Artifacts
- The authored Rust integration test constructs the real Copilot provider and sidecar without contacting an external service, then checks that live effort remains high.
- The executed test passed with memory effort unset and live effort still high, establishing the comparison condition.
- The executed test failed because sidecar construction changed live effort from high to low, reproducing the defect.
Prompt To Fix With AI
This is a comment left during a code review.
Path: crates/jcode-base/src/sidecar.rs
Line: 197-203
Comment:
**Sidecar changes live effort**
When the active provider is Copilot `claude-sonnet-5`, its fork shares the live provider’s effort setting. Setting `agents.memory_effort` here changes the effort used by subsequent main-agent requests instead of isolating it to memory extraction. This must be fixed before merging.
**Knowledge Base Used:** [Model provider integration](https://app.greptile.com/solo-systems/-/custom-context/knowledge-base/1jehuang/jcode/-/docs/provider-integration.md)
---
For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.
Comment on lines
+1195
to
+1198
| let budget = budget.min(max_tokens.saturating_sub(1)); | ||
| if budget < 1_024 { | ||
| None | ||
| } else { |
There was a problem hiding this comment.
With claude-sonnet-3-7 and the sidecar’s default 1,024-token output limit, the thinking budget falls below its minimum. The request then contains neither thinking nor an output_config effort setting. The configured memory effort has no effect for this model; this narrower configuration defect does not independently block merging.
Knowledge Base Used: Model provider integration
Artifacts
- This authored command extracts and compiles the parent or current request code and prints its serialized bodies, making the two runs reproducible.
- The parent-code harness ran successfully and emitted requests without reasoning fields at all three tested token limits, establishing the baseline.
- The current-code harness ran successfully and showed effort omitted at 1,024 tokens but manual thinking present at 1,025 and 2,000 tokens, confirming the clamp boundary.
- The focused Cargo test passed; it does not cover the manual-only Sonnet 3.7 default-limit case.
Prompt To Fix With AI
This is a comment left during a code review.
Path: crates/jcode-base/src/sidecar.rs
Line: 1195-1198
Comment:
**Claude effort is omitted**
With `claude-sonnet-3-7` and the sidecar’s default 1,024-token output limit, the thinking budget falls below its minimum. The request then contains neither `thinking` nor an `output_config` effort setting. The configured memory effort has no effect for this model; this narrower configuration defect does not independently block merging.
**Knowledge Base Used:** [Model provider integration](https://app.greptile.com/solo-systems/-/custom-context/knowledge-base/1jehuang/jcode/-/docs/provider-integration.md)
---
For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.
Comment on lines
+1652
to
+1655
| swarm_model_override: crate::config::config().agents.swarm_model.clone(), | ||
| swarm_model_effort: crate::config::config().agents.swarm_effort.clone(), | ||
| memory_model_override: crate::config::config().agents.memory_model.clone(), | ||
| memory_model_effort: crate::config::config().agents.memory_effort.clone(), |
There was a problem hiding this comment.
In an SSH session, the selected model and effort reflect remote state, but these swarm and memory rows read the client’s local configuration. When the hosts differ, Runtime shows the wrong overrides or omits ones used by the server. This makes remote settings harder to diagnose but does not independently block merging.
Knowledge Base Used: Interactive user interfaces
Artifacts
- The temporary Rust test defines differing client and simulated server settings and renders the actual widget; it exercises the disputed data path.
- The runner temporarily includes and executes the Rust test for both client configurations, then restores the source file; no product edit remains.
- The first Rust test passed and rendered the server-selected model without agent override rows; the differing simulated server overrides were not displayed.
- The second Rust test passed and rendered client swarm and memory overrides beside the server-selected model; the remote panel mixed settings sources.
- Chromium recorded playback of the first passing Rust terminal buffer; only the server-selected model appears.
Runtime panel with client overrides unset
- Chromium captured the first passing Rust terminal buffer; agent override rows are absent.
- Chromium recorded playback of the second passing Rust terminal buffer; client override rows appear beside the server-selected model.
Runtime panel displaying client overrides beside server selection
- Chromium captured the second passing Rust terminal buffer; the agent rows show client values rather than the simulated server values.
- The script replays the successful Rust terminal buffers in Chromium and captures matching videos and poster frames; the images depict executed test output.
- The recorded command exited successfully and produced both playback videos and poster frames; visual capture completed.
Prompt To Fix With AI
This is a comment left during a code review.
Path: crates/jcode-tui/src/tui/app/tui_state.rs
Line: 1652-1655
Comment:
**Remote panel shows local overrides**
In an SSH session, the selected model and effort reflect remote state, but these swarm and memory rows read the client’s local configuration. When the hosts differ, Runtime shows the wrong overrides or omits ones used by the server. This makes remote settings harder to diagnose but does not independently block merging.
**Knowledge Base Used:** [Interactive user interfaces](https://app.greptile.com/solo-systems/-/custom-context/knowledge-base/1jehuang/jcode/-/docs/interactive-user-interfaces.md)
---
For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.
Comment on lines
+1470
to
+1473
| let _guard = crate::storage::lock_test_env(); | ||
| let dir = tempfile::TempDir::new().expect("tempdir"); | ||
| crate::env::set_var("JCODE_HOME", dir.path()); | ||
| std::fs::write( |
There was a problem hiding this comment.
This test changes the process-wide JCODE_HOME without restoring it. After its temporary directory is removed, later tests in the same process can resolve configuration or storage through a deleted path. The resulting order-dependent test failures are a non-blocking test-isolation concern.
Artifacts
- This authored command builds the observer and runs each real test with the same initial home, producing the comparison logs.
- This authored observer runs as the test process exits and checks its final JCODE_HOME, making the post-test state visible.
- The comparable guarded test passed, and its process exited with the original JCODE_HOME still pointing to an existing directory.
- The candidate test passed, but its process exited with JCODE_HOME pointing to a deleted temporary directory.
Prompt To Fix With AI
This is a comment left during a code review.
Path: crates/jcode-base/src/sidecar.rs
Line: 1470-1473
Comment:
**Test leaves deleted home**
This test changes the process-wide `JCODE_HOME` without restoring it. After its temporary directory is removed, later tests in the same process can resolve configuration or storage through a deleted path. The resulting order-dependent test failures are a non-blocking test-isolation concern.
---
For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.
Comment on lines
+124
to
+126
| if let Some(model) = non_empty(data.memory_model_override.as_deref()) { | ||
| let mut text = model.to_string(); | ||
| if let Some(effort) = non_empty(data.memory_model_effort.as_deref()) { |
There was a problem hiding this comment.
JCODE_MEMORY_EFFORT can set sidecar effort without a memory-model override, but Runtime renders the memory row only when a model override exists. Users with an effort-only setting cannot verify it in the panel; this visibility gap does not independently block merging.
Artifacts
- This authored command temporarily adds and runs the focused render test, then restores the source file; it is the executable reproduction.
- The executed test prints all three rendered panels and passes its assertions; the effort-only panel matches the baseline.
- This authored Playwright script displays the cells captured by the Rust test and records the comparison; it does not invent UI output.
- The executed Playwright command reports successful baseline and effort-only captures; both lack a memory row.
- Chromium displays the baseline Rust-rendered panel without a memory override; it contains only the session model row.
Poster of the baseline runtime panel
- The captured baseline frame shows the session model row and no memory row.
- Chromium displays the Rust-rendered panel with `JCODE_MEMORY_EFFORT=low` and no memory model override; the memory row is still absent.
Poster of the effort-only runtime panel
- The captured effort-only frame looks like the baseline despite the configured sidecar effort.
Prompt To Fix With AI
This is a comment left during a code review.
Path: crates/jcode-tui/src/tui/info_widget_model.rs
Line: 124-126
Comment:
**Effort-only setting stays hidden**
`JCODE_MEMORY_EFFORT` can set sidecar effort without a memory-model override, but Runtime renders the memory row only when a model override exists. Users with an effort-only setting cannot verify it in the panel; this visibility gap does not independently block merging.
---
For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.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!
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #1639. Closes #1634.
Three related info-panel changes in one commit:
agents.memory_effort(envJCODE_MEMORY_EFFORT) pins the reasoning effort of the memory extraction sidecar on every backend: OpenAI reasoning pin, Claude thinking budget or output_config effort derived from provider-core caps, or set_reasoning_effort on the sidecar's independent provider fork. Empty strings trim to unset; runtime panel shows it next to the memory model row (#1639).The runtime panel gains rows for the selected model and
agents.*model overrides (#1634).The separate Swarm, Commits and Compaction margin widgets fold into one always-visible Overview (order: Runtime, Todos, Memory, Background, Usage, KV, Compaction, Changes, Commits, Swarm). Includes a placement fix: mergeable widgets could anchor into the only margin pocket before the Overview had data and squat it indefinitely; placement now retries once without mergeable anchors when the Overview is available but unplaced. MemoryActivity stays a dedicated widget since the Overview strips memory_info.
Tests: 245 new/expanded info-widget tests cover folding order, runtime rows, memory_effort plumbing and placement retry, plus placement-state isolation locks (the widget placement state is process-global). Full-suite failures observed locally are the known upstream flakes (restore_session, ambient refresh, model picker copilot, cost_based_usage, state_persists_show_count), all reproducible on current master. Single commit rebased onto current master.