Refactor base and app-core to remove dead TUI dependencies#400
Open
quangdang46 wants to merge 42 commits into
Open
Refactor base and app-core to remove dead TUI dependencies#400quangdang46 wants to merge 42 commits into
quangdang46 wants to merge 42 commits into
Conversation
…nds on TUI crates jcode-base was a 'foundation' layer that declared 11 jcode-tui-* dependencies plus ratatui/arboard, even though its source used only two pure symbols from them: ResumeTarget (a plain data enum) and reasoning_line_markup (a pure string formatter). The other 9 tui deps were dead edges. Move the two symbols to where they belong: - ResumeTarget -> jcode-session-types (pure data; session-picker re-exports it) - reasoning markup helpers + REASONING_SENTINEL -> jcode-render-core (pure/backend-neutral; tui-markdown re-exports them) Then drop all 11 jcode-tui-* deps + ratatui + arboard from jcode-base and add jcode-render-core. Public paths (jcode_tui_session_picker::ResumeTarget, jcode_tui_markdown::reasoning_line_markup) keep working via re-exports. The foundation no longer depends on any presentation crate, breaking the base->tui->base style layering inversion and shrinking base's compile graph. No behavior change; validated by full jcode binary build + render-core/ session-types/tui-markdown/base import+reasoning tests.
jcode-app-core declared all 11 jcode-tui-* crates but used none of them in its source (verified: 0 references), and did not re-export them. They were dead edges left over from the base/app-core/tui split. The TUI crate declares these deps itself. Removing them means editing a jcode-tui-* crate no longer cascades a recompile through app-core (measured: touching jcode-tui-style now rebuilds only tui-style -> tui -> jcode, ~6s, instead of also rebuilding base+app-core). No behavior change; full jcode binary builds clean.
…is populated The portable Linux release is built inside a Docker container that bind-mounts the repo (owned by the host UID) and runs git as root. Every in-container git call trips git's dubious-ownership guard (CVE-2022-24765) and fails, which zeroed out the embedded git hash, date, AND changelog. Shipped binaries reported "vX.Y.Z (unknown) (unknown)" with an empty /changelog overlay. Compute git hash/date/tag/dirty/changelog_raw on the host and pass them into the container via JCODE_BUILD_METADATA_FILE (same mechanism as remote_build.sh), and add 'git config --global --add safe.directory /work' as a fallback. Verified: with git failing in-container the metadata file alone now embeds 700 changelog entries and the correct version string.
Brings in two refactors that break the foundation-depends-on-presentation layering inversion and prune dead dependency edges: - base: move ResumeTarget -> jcode-session-types and reasoning markup helpers + REASONING_SENTINEL -> jcode-render-core; drop all 11 jcode-tui-* deps plus ratatui/arboard from jcode-base. Public paths preserved via re-exports. - app-core: drop 11 unused jcode-tui-* dependency edges so editing a tui crate no longer cascades a recompile through app-core. Conflict resolution: master added a 'current' reasoning-display mode that called jcode_tui_markdown::reasoning_summary_line_markup from jcode-base. Moved that pure helper into jcode-render-core too (re-exported from tui-markdown) so base stays free of tui deps. Switched new base test REASONING_SENTINEL imports to jcode_render_core. Validated: full jcode binary builds clean; render-core/session-types/tui-markdown tests pass; base reasoning render tests pass.
… decision
The OAuth-vs-API-key decision for the two dual-auth providers (Anthropic/
Claude and OpenAI) was encoded as free-form strings across four overlapping
vocabularies (runtime env `JCODE_RUNTIME_PROVIDER`, route stable-id, CLI
`--provider`, and model prefix). ~15 call sites each hand-maintained their
own subset of aliases, and those subsets drifted:
- `OpenAICredentialMode::from_runtime_env` accepted `openai` but not the
route-vocabulary `openai-oauth`.
- `AnthropicCredentialMode::from_runtime_env` accepted neither `claude-oauth`
nor `anthropic-api-key`.
- pricing, billing, header tag, session-key folding, swarm-spawn routing, and
CLI-arg translation each re-derived the decision independently.
When a string from one vocabulary leaked into a parser that only knew another,
OAuth and API-key paths got mixed up.
This adds `jcode_provider_core::auth_mode` as the canonical source of truth:
`AuthRoute { provider, mode }` parses *any* alias from *any* vocabulary and
emits the canonical string for each vocabulary (runtime key, route api_method,
model prefix, session key, CLI arg). Every scattered parser/emitter now routes
through it:
- `ModelRouteApiMethod::parse` delegates its dual-auth cases.
- Anthropic/OpenAI `from_runtime_env` + credential-mode writers.
- `resolve_dual_credential_auth` (header tag / info widget / model-switch line).
- `update_cost_impl` billing decision.
- `cli_provider_arg_for_session_key`, `canonical_session_provider_key`,
`explicit_session_provider_key_for_model_request`,
`model_switch_request_for_session_model`, `fork_model_switch_request`.
- `cheapness_for_route` pricing and swarm `explicit_route_for_configured_model`.
`parse_explicit_credential_prefix` preserves the model-prefix nuance that bare
`claude:`/`openai:` route without pinning a credential (vs runtime/CLI where
bare `claude` means OAuth).
Also fixes a stale test that asserted Anthropic Auto was API-key-first; the
codebase consistently implements OAuth-first Auto (matching OpenAI and
`resolve_dual_credential_auth`).
Adds cross-vocabulary round-trip tests proving every alias resolves
consistently and agrees with `ModelRouteApiMethod`.
In 'current' reasoning-display mode each closed reasoning block was moved into a dedicated persistent "reasoning" display message that collapsed to a '> thought' trace and never went away. This caused three visible bugs: - Reasoning accumulated: every block left a stacked trace line, so the transcript showed all past reasoning instead of just the current one. - Interleaved answer/reasoning/answer was reordered: extracting the block mid-stream floated reasoning above still-uncommitted answer text and merged the pre- and post-reasoning paragraphs. - Live vs reloaded rendering diverged (separate block + 'for Ns' summary live; inline assistant text + '(N lines)' summary on reload). Now 'current' reasoning is ephemeral: it streams live as dim+italic text, then is sliced straight back out of the stream in place once the model starts answering or runs a tool. Only the live block is ever shown, answer order is preserved, and nothing accumulates. Reloaded history hides past reasoning to match. Removes the ReasoningCollapse animation machinery. Tests updated to cover ordering preservation and non-accumulation.
Capture the response usage.service_tier so we can confirm whether the priority/auto request was actually honored or silently fell back to standard (set JCODE_LOG_SERVICE_TIER=1 to print it). Add 429 retry and a cooldown gap to the essay TPS benchmark.
Introduce FileTouchService owning the two file-touch maps (path->accesses and session->paths) behind intention-revealing async methods (record_touch, accesses_for_path, sorted_file_strings_for_session, snapshot, reverse_snapshot, clear_session, expire_older_than). Replace the two raw Arc<RwLock<..>> fields on Server with a single file_touch: FileTouchService field, thread the handle through ServerRuntime/handle_client/handle_debug_client instead of passing the raw maps, and route all non-test server/* call sites through the service. Removes the now-unused swarm::remove_session_file_touches helper. Behavior-preserving: same locking order and semantics retained.
Update test setup in client_comm_tests, client_lifecycle_tests, and the client_session_tests suite to construct FileTouchService::new() and pass the handle instead of the raw file_touches/files_touched_by_session maps.
Replace static WORKSPACE_STATE Mutex<Option<WorkspaceClientState>> with a WorkspaceClientState field on App. Convert the module's free functions to methods on the struct. Each client instance now owns its workspace map instead of sharing one process-global, eliminating cross-instance state leakage. Behavior-preserving; updates all ~7 non-test callers plus tests.
The merged-in cache-write cost test (master) used pre-rename App field names; route them through the StreamingProgress/CostState sub-structs.
Add docs/TUISTATE_TRAIT_DECOMPOSITION.md categorizing all 114 TuiState methods into 15 domains with an incremental, low-conflict split plan, and annotate the trait definition with matching section headers. Behavior-neutral (comments only); compiles clean.
Add BENCH_ANTHROPIC_API_KEY=1 to pin the direct Console API-key credential (x-api-key) instead of the subscription OAuth route, and log the resolved key prefix under JCODE_LOG_SERVICE_TIER so we can confirm which credential/tier was actually used. Both OAuth and API-key paths return service_tier=standard on this account (no fast-mode credits).
The niri `jcode self-dev` hotkey (Alt+Semicolon) launches via run_self_dev, which jumps straight to run_tui_client and bypasses maybe_show_setup_hints. That means launch_count in setup_hints.json never advances, so the is_new_user_for_onboarding() heuristic (launch_count <= 5) treats every self-dev spawn as a brand-new user and re-runs onboarding each time. Guard both onboarding entry points on is_selfdev_canary_session(): canary sessions are developers working on jcode, not first-run users, so they should never auto-start the guided flow. /onboarding-preview still works for testing.
…ket fallback A naive substring check (is_websocket_fallback_notice) treated any SSE payload containing 'falling back from websockets to https transport' as a plain-text transport control frame and dropped it. When the model edited source files that mention that phrase, the text rode along inside structured response.completed / response.output_item.done / response.function_call_arguments.done events, all of which were silently dropped. MessageEnd was never emitted, surfacing as 'OpenAI HTTPS stream ended before message completion marker' and forcing repeated retries. Structured response.*/error events are now never reinterpreted as plain transport control frames. Same guard applied to the 'stream disconnected before completion' check. Adds regression tests.
The onboarding welcome screen animates a donut, so it redraws at animation FPS. Each redraw calls suggestion_prompts() (twice: once via onboarding_welcome_active() and again via welcome_body_lines()), which calls latest_external_cli_continuation_prompt(). That function reads and JSON-parses the newest 32 transcripts from ~/.codex/sessions and ~/.claude/projects on every call. For users with large external histories this is very expensive: measured ~387ms per call against a ~1.2GB ~/.codex/sessions tree (parsing ~17k JSON lines / ~39MB per frame), running ~2x per frame -> ~0.7s of blocking work per onboarding frame, which is the source of the onboarding lag. Wrap the scan in a 30s TTL cache. The cached path drops repeated calls from ~387ms to ~0.0001ms. Adds an ignored timing test (onboarding_suggestion_scan_cost) documenting cold vs warm cost.
…f-dev pins When a long-lived daemon is already on the newest release, the /update no-op path never re-ran the install path that advances the shared-server channel, leaving the daemon pinned to an older binary forever. Repair the shared-server channel after no-op update checks (both hot_exec and the update module) and request a forced server reload so the daemon execs into a strictly-newer binary. Guard the repair with is_release_channel_marker so it only overwrites release-channel markers and never clobbers a deliberately-pinned self-dev shared-server build, even when that pin is older than stable.
Live reasoning now surfaces as a 'thinking…' status instead of 'streaming…'. Cover the remote ReasoningDelta path: it flips the status to Thinking, a subsequent TextDelta returns to Streaming, and a running tool is never masked by reasoning text.
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
2 participants
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.
No description provided.