diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml new file mode 100644 index 0000000..e395f20 --- /dev/null +++ b/.github/workflows/ci.yml @@ -0,0 +1,45 @@ +name: CI + +on: + push: + branches: [main] + pull_request: + workflow_dispatch: + +permissions: + contents: read + +jobs: + build: + # OpenKey targets net10.0-windows and uses DPAPI, so it cannot build or test on Linux. + runs-on: windows-latest + + steps: + - uses: actions/checkout@v4 + + # No version here: global.json pins the SDK, so CI and a developer machine agree. + - uses: actions/setup-dotnet@v4 + + - name: Restore + run: dotnet restore + + # Directory.Build.props sets TreatWarningsAsErrors, and IsAotCompatible turns the + # trim/AOT analyzers on, so this is a strict gate rather than a formality. + - name: Build + run: dotnet build --no-restore -c Release + + - name: Test + run: dotnet test --no-build -c Release --verbosity normal + + # Proves the publish profile in OpenKey.csproj still produces the shipping artifact + # without anyone pasting flags from a README. + - name: Verify single-file publish + run: dotnet publish src/OpenKey/OpenKey.csproj -c Release -r win-x64 -o publish-check + + - name: Confirm the exe exists + shell: pwsh + run: | + $exe = "publish-check/OpenKey.exe" + if (-not (Test-Path $exe)) { throw "Expected $exe to exist" } + $mb = [math]::Round((Get-Item $exe).Length / 1MB, 1) + Write-Host "OpenKey.exe is $mb MB" diff --git a/.github/workflows/release.yml b/.github/workflows/release.yml new file mode 100644 index 0000000..c560893 --- /dev/null +++ b/.github/workflows/release.yml @@ -0,0 +1,48 @@ +name: Release + +on: + push: + tags: ["v*"] + workflow_dispatch: + +permissions: + contents: write + +jobs: + publish: + runs-on: windows-latest + + strategy: + matrix: + rid: [win-x64, win-arm64] + + steps: + - uses: actions/checkout@v4 + - uses: actions/setup-dotnet@v4 + + - name: Test before shipping + run: dotnet test -c Release + + # All publish flags live in OpenKey.csproj, so this line cannot drift from what was tested. + - name: Publish ${{ matrix.rid }} + run: dotnet publish src/OpenKey/OpenKey.csproj -c Release -r ${{ matrix.rid }} -o out/${{ matrix.rid }} + + - name: Name the artifact by platform + shell: pwsh + run: | + New-Item -ItemType Directory -Force dist | Out-Null + Copy-Item "out/${{ matrix.rid }}/OpenKey.exe" "dist/OpenKey-${{ matrix.rid }}.exe" + + - uses: actions/upload-artifact@v4 + with: + name: OpenKey-${{ matrix.rid }} + path: dist/OpenKey-${{ matrix.rid }}.exe + + # Release titles carry the version only — never a phase number. See docs/06. + - name: Attach to the release + if: startsWith(github.ref, 'refs/tags/v') + uses: softprops/action-gh-release@v2 + with: + name: ${{ github.ref_name }} + files: dist/OpenKey-${{ matrix.rid }}.exe + generate_release_notes: true diff --git a/BACKLOG.md b/BACKLOG.md new file mode 100644 index 0000000..e39aa04 --- /dev/null +++ b/BACKLOG.md @@ -0,0 +1,121 @@ +# Backlog + +**What this file is for.** `docs/07-roadmap.md` says *what* is planned and *why*, organised by +tier. This file says *what state each item is in* and *what order it happens in*. Entries here link +to the roadmap rather than restating it — one canonical home per fact. + +Tier numbers refer to the ladder in [`docs/07-roadmap.md`](docs/07-roadmap.md#tier-ladder--sort-rule). + +--- + +## Done + +### Production readiness pass — 2026-08-03 + +- Dependency upgrade: Spectre.Console 0.49.1 → 0.57.2, Markdig, Microsoft.Extensions, test + packages. SDK pinned via `global.json`; `win-arm64` added. +- Publish profile moved from README prose into `OpenKey.csproj`. +- `System.Text.Json` source generation for everything persisted and every request body. +- 14 correctness defects fixed — see [`CHANGELOG.md`](CHANGELOG.md) for the user-facing list. +- Console rebuilt: block-level streaming, `Theme`/`Glyphs`/`Components`, error cards, sentence-case + voice, ASCII glyph fallback, exit hold. +- Test suite 11 → 65, including the first coverage `ChatEngine` has ever had. +- Docs: `docs/architecture/`, user guide, testing guide, all known doc/code contradictions + resolved. +- Repo hygiene: `LICENSE` (MIT), `CHANGELOG.md`, `CONTRIBUTING.md`, `SECURITY.md`, this file, CI + and release workflows. + +### Tier 2 and Tier 3 backlog, plus roadmap Quick Wins — 2026-08-03 + +- **`/new`** — start a fresh conversation, keeping the key. Was the most conspicuous missing verb: + clearing history previously meant `/reset`, which also deleted the key. +- **`/retry`** — resend the last message. Routed back through the host so a resend takes exactly + the same path as a typed message. +- **`/history`**, **`/export [path]`** (defaults to a timestamped file on the Desktop), + **`/copy`** (via `clip.exe` — a console app has no clipboard API without a UI framework). +- **`/theme default|dark|light|mono`**, persisted. +- **`config.json`** — `IConfigStore` / `JsonConfigStore`, matching the shape already documented in + `docs/05`. A pinned model now survives a restart, which it never could before because there was + nowhere to store it. Hand-edited values are normalised rather than trusted. +- **Real tokenizer** — `ITokenCounter` in Core (so Core keeps its zero package references) with a + cl100k-backed implementation in the host. Vocabulary embedded, not downloaded: OpenKey must work + on first run behind a captive portal and makes no network call except to OpenRouter. +- **OAuth port fallback** — four known callback ports tried in order instead of only 3000. Fixed + URLs, not random ones, since OpenRouter 409s on a varying callback. +- **Whole-turn budget** — two minutes across all attempts, checked *between* attempts only: a reply + that is actively arriving is working, however long it has taken. +- **Link URLs escaped** rather than bracket-filtered, which used to silently drop the target of any + URL containing a bracket. +- **Accessibility pass** — verified colour is never the only signal (`✓`/`✗` differ, error cards + name the problem in their title), with the `mono` palette as the standing test and a unit test + asserting no hue survives it. + +Two items were resolved differently from how they were written, both noted here because the +deviation is the point: + +- **`/stop` was not added.** A command cannot work while a reply streams — the app is not reading a + prompt — and Ctrl+C already cancels correctly. The real gap was that nothing said so, so the fix + is a one-off `(Ctrl+C to stop)` hint. A key-watcher was considered and rejected: it would swallow + type-ahead, and people routinely start composing the next message while a reply arrives. +- **Banner artwork was not added.** The roadmap asked for richer ASCII art; the console design + principle is that calm beats decorative, and the banner is the first thing a non-technical user + sees. Adding art would contradict the design it is supposed to serve. + +Also fixed en route: `Microsoft.ML.Tokenizers` 2.0.0 pulls in `Microsoft.Bcl.Memory` 9.0.4, which +carries a known high-severity advisory (GHSA-73j8-2gch-69rq). NuGet audit failed the build; pinned +forward to 10.0.10. + +### v0.1.0 — 2026-05-28 + +First release. See [`CHANGELOG.md`](CHANGELOG.md#010--2026-05-28). + +--- + +## Next up + +Ordered by user value within tier. Lowest tier wins. + +Tier 2 and Tier 3 are complete — see **Done** above. What remains is Tier 4, which is Phase 5 work +and a step change in scope rather than more polish. + +### Tier 4 — providers + +13. **Anthropic provider** — see [roadmap Phase 5](docs/07-roadmap.md#phase-5--claude-code-provider-integration). + Worth building *before* the Claude Code subprocess provider: `IChatProvider.Id` and + `DisplayName` are currently never read by anything, so the multi-provider seam has never been + exercised. A second real provider is what proves the abstraction is right. +14. **Adopt `Microsoft.Extensions.AI` beneath `IChatProvider`** — `IChatClient` would supply + tool-calling middleware for Phase 3 and a wide provider ecosystem. It must sit *under* our + interface, never replace it: it is .NET-only and a browser or Android port could not implement + it. Rationale in + [`docs/architecture/08-decisions.md`](docs/architecture/08-decisions.md). + +--- + +## Watching + +Not scheduled; revisit when the trigger fires. + +- **NativeAOT** — would cut the binary from ~42 MB to roughly 15–20 MB, remove the extract-to-temp + step on first run, and start faster. All three matter for the USB story. The old blocker + (Spectre reflection) is gone as of 0.55, and JSON source generation has landed, so the remaining + cost is measuring what the analyzers still report. `IsAotCompatible` is already on and the tree + is warning-clean. +- **`System.Net.ServerSentEvents`** — would replace the hand-rolled SSE reader. Preview-only today + (`11.0.0-preview.6`); adopt when it ships stable. +- **Bracketed paste** — a more robust multi-line paste than the current timing heuristic. Needs a + custom input reader and is Windows-Terminal-only, so it is not worth it yet. + +--- + +## Deliberately not doing + +Recorded so they are not proposed again. Full reasoning in +[`docs/architecture/08-decisions.md`](docs/architecture/08-decisions.md). + +- `LiveDisplay` for the transcript — it destroys scrollback. +- `IHttpClientFactory` or Polly — transport retries would corrupt rotation's cooldown accounting. +- `Microsoft.Extensions.Hosting` — a REPL does not need a generic host. +- Paid models in any 1.x release. +- Telemetry, in any phase. +- An auto-update installer. Check-and-notify only. diff --git a/CHANGELOG.md b/CHANGELOG.md new file mode 100644 index 0000000..e04ddea --- /dev/null +++ b/CHANGELOG.md @@ -0,0 +1,102 @@ +# Changelog + +Notable changes to OpenKey. Format follows [Keep a Changelog](https://keepachangelog.com/en/1.1.0/); +versions follow [Semantic Versioning](https://semver.org/spec/v2.0.0.html). + +## [0.2.0] — 2026-08-03 + +### Added + +- `/new` starts a fresh conversation while keeping you signed in. Previously the only way to clear + history was `/reset`, which also deleted your key. +- `/retry` resends your last message; `/history` shows the conversation; `/copy` puts the last + reply on the clipboard; `/export` saves it as markdown, defaulting to your Desktop. +- `/theme default|dark|light|mono`, remembered between runs. `mono` drops colour entirely for + high-contrast setups or screenshots. +- Preferences are saved, so a model chosen with `/models` now survives a restart. +- Token counting uses a real tokenizer instead of a character estimate, so conversations are + trimmed more accurately as they grow. +- Browser sign-in falls back across several local ports instead of giving up when one is taken. +- Replies stream as they arrive. Each completed markdown block is rendered styled, so code fences + become panels and prose keeps its emphasis, while the still-arriving tail stays plain. +- Reply header showing which model answered and how long it took. +- Error cards that say what happened and what to do next, replacing raw error-kind names. +- `/models` shows model names and context sizes instead of raw ids. +- OpenKey holds the window open on exit and on failure when it was double-clicked, so parting + messages and errors are actually readable. +- Line editing and history at the prompt, courtesy of the Windows console reader. +- Multi-line paste is kept as one message. +- MIT `LICENSE`, `CONTRIBUTING.md`, `SECURITY.md`, `BACKLOG.md`, and GitHub Actions for CI and + releases. +- Architecture reference under `docs/architecture/`, a user guide, and a testing guide. + +### Fixed + +- Links whose address contained a bracket lost their target when displayed. +- A message that kept failing could retry for several minutes; it is now bounded, and a reply + that is genuinely arriving is never cut off. +- **Long replies from slow models always failed.** The HTTP timeout covered reading the response + body, so a healthy reply that took over 60 seconds was aborted, misread as a network fault, and + retried on another model that failed the same way. Deadlines now bound the wait for the next + token rather than the whole reply. +- **Replies were never saved.** Session persistence ran after the final chunk was yielded, and the + console stops reading at that point, so the code never executed. No conversation was ever + written to disk. +- **Resuming a conversation never worked.** `ChatMessage` had two constructors, so deserialization + threw an error the session store did not catch. +- **Public Wi-Fi sign-in pages crashed the app.** A captive portal replies to any request with HTML + and HTTP 200; parsing that threw, and with no top-level handler the window closed on the stack + trace. +- **One Ctrl+C disabled the session.** A single cancellation source lived for the whole process, so + after the first Ctrl+C every later message was cancelled before it started. +- **A mid-reply model switch showed the answer twice**, concatenated, while the saved history + stored it once. +- **`/reset` could strand you.** Abandoning setup left OpenKey running with no key: every message + failed, the error suggested `/reset`, and `/reset` returned to the same place. +- **An empty model list disabled the app for 24 hours** — it was cached with a full-day lifetime and + then crashed on every launch, unrecoverable without deleting `%APPDATA%\OpenKey` by hand. +- Replies that ended without an explicit completion signal were discarded as broken and retried, + even though they were complete. +- Free models priced as `0.000000` were misread as paid and hidden from `/models`. +- Cancelled and failed messages stayed in the conversation history and were saved later. +- A network outage put every model on cooldown, so OpenKey stayed broken after the network came + back. +- Requests a model cannot accept are no longer retried across every other model. +- Disk failures during a save no longer crash the app after a reply has been generated. +- Inline code was styled so that it was invisible on light terminals and indistinguishable from + body text on dark ones. +- Markdown blocks ran together with no spacing between them. +- Glyphs that render as boxes in the classic Windows console — including the spinner, which + appeared on every message — now fall back to plain ASCII. +- Output wider than 100 columns no longer stretches code blocks across the whole screen. +- Redirecting output to a file no longer exits the app immediately. + +### Changed + +- Interface language moved to plain sentence case; no screen shows acronyms like DPAPI or OAuth, + or internal error names. +- Model rotation is reported as one quiet line instead of a warning for each attempt. +- `/reset` states plainly that it erases the key *and* the conversation before asking to confirm. +- Dependencies updated: Spectre.Console 0.49.1 → 0.57.2, Markdig 1.2.0 → 1.3.2, and the + Microsoft.Extensions and test packages to their current releases. +- Publish settings moved into the project file, so a plain `dotnet publish` produces the shipping + binary. +- Windows on ARM (`win-arm64`) is built alongside `win-x64`. +- Test coverage grew from 11 tests to 84. +- A dependency carrying a known high-severity advisory (`Microsoft.Bcl.Memory` 9.0.4, pulled in + transitively) was pinned forward before it could ship. + +## [0.1.0] — 2026-05-28 + +### Added + +- First release. Windows console chat client for free OpenRouter models. +- Browser sign-in (PKCE) or paste an existing key; the key is encrypted for your Windows account. +- Automatic rotation across free models when one is rate-limited, with cooldown tracking. +- Conversation history and model cache under `%APPDATA%\OpenKey\`. +- Commands: `/about`, `/models`, `/model`, `/cls`, `/help`, `/reset`, `/quit`. +- Single self-contained `.exe` that runs from a USB stick with nothing installed. + +[Unreleased]: https://github.com/corecompiled/OpenKey/compare/v0.2.0...HEAD +[0.2.0]: https://github.com/corecompiled/OpenKey/releases/tag/v0.2.0 +[0.1.0]: https://github.com/corecompiled/OpenKey/releases/tag/v0.1.0 diff --git a/CLAUDE.md b/CLAUDE.md index 987cbf3..5bdf5da 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -47,18 +47,12 @@ When the user mentions a new feature / QoL / roadmap item, slot it into existing 2. **If it's structurally new**, add a new section in `docs/07-roadmap.md` at the correct tier position. 3. **If it doesn't fit any tier cleanly**, add it to the **Quick Wins** flat list at the bottom of `docs/07-roadmap.md`. -Sort by the **tier ladder**: - -| Tier | Meaning | -|------|---------| -| 1 | Exe-blocking (must ship for Phase 1) | -| 2 | Exe polish (Phase 1.1 / 1.2) | -| 3 | Current-UI features within an existing host | -| 4 | New providers (no UI change) | -| 5 | New UI surfaces (GUI, PWA, Android) | +Sort by the **tier ladder**, which is defined once in [`docs/07-roadmap.md`](docs/07-roadmap.md#tier-ladder--sort-rule). Don't restate it here — a copy in this file previously drifted out of sync with the canonical one, which is exactly what rule 5 of the hygiene checklist exists to prevent. New items slot into the **lowest tier they legitimately belong to**, ordered within tier by user value. **One canonical home per item** — never silently duplicate across docs. +**Roadmap vs backlog.** `docs/07-roadmap.md` says *what* and *why*, grouped by tier. [`BACKLOG.md`](BACKLOG.md) says *what state* each item is in and *in what order* it happens. Backlog entries link to the roadmap rather than restating it. + ## Cross-UI guarantee All UIs (desktop console, future Avalonia GUI, future PWA, future Android APK) implement the **same** behavior defined in the contract docs. @@ -94,6 +88,11 @@ If any item fails, fix before reporting the task done. Surface unresolvable conf | File | Purpose | Contract? | |------|---------|-----------| | `CLAUDE.md` (this file) | Meta-rules for Claude sessions | no | +| `README.md` | Front page: what it is, quick start, doc index | no | +| `BACKLOG.md` | Execution state and order (roadmap says what/why) | no | +| `CHANGELOG.md` | Released changes, Keep a Changelog format | no | +| `CONTRIBUTING.md` | Build, test, house rules, contract-change process | no | +| `SECURITY.md` | Reporting, data handling, threat model | no | | `docs/00-overview.md` | Pitch, principles, phase ladder, glossary | no | | `docs/01-architecture.md` | Layers, `IChatProvider`, error taxonomy, cross-UI contract | **yes** | | `docs/02-phase1-build.md` | Step-by-step Phase 1 build walkthrough | no (impl guide) | @@ -102,6 +101,12 @@ If any item fails, fix before reporting the task done. Surface unresolvable conf | `docs/05-persistence-and-reset.md` | `%APPDATA%` layout, DPAPI, `/reset` | **yes** | | `docs/06-build-and-distribute.md` | `dotnet publish`, smoke test, USB distribution | no | | `docs/07-roadmap.md` | Future phases, tier ladder, Quick Wins | no | +| `docs/08-user-guide.md` | End-user manual: commands, troubleshooting, FAQ | no | +| `docs/09-testing.md` | Test layout, helpers, conventions | no | +| `docs/architecture/` | Explanatory deep-dives; `01` stays normative over all of them | no | +| `docs/architecture/08-decisions.md` | Decisions **and explicit rejections**, with evidence | no | + +Before proposing a library, a pattern, or an approach, check `docs/architecture/08-decisions.md` — several obvious-looking options were evaluated and rejected there for concrete, recorded reasons. ## Things never to do diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md new file mode 100644 index 0000000..3f1d410 --- /dev/null +++ b/CONTRIBUTING.md @@ -0,0 +1,92 @@ +# Contributing + +## Getting a build + +```cmd +git clone https://github.com/corecompiled/OpenKey.git +cd OpenKey +dotnet build +dotnet test +``` + +You need the .NET SDK version pinned in `global.json` (or a later patch of the same feature band). +Windows only — OpenKey targets `net10.0-windows` and uses DPAPI for key storage. + +Run it from source: + +```cmd +dotnet run --project src\OpenKey\OpenKey.csproj +``` + +Build the shipping binary: + +```cmd +dotnet publish src\OpenKey\OpenKey.csproj -c Release -r win-x64 -o publish\ +``` + +Every publish flag lives in `OpenKey.csproj`. Don't pass them on the command line, and don't +document a different command anywhere — the point is that CI and a developer machine produce the +same artifact. See [`docs/06-build-and-distribute.md`](docs/06-build-and-distribute.md). + +## Before you open a PR + +- `dotnet build` is clean. `TreatWarningsAsErrors` is on, and `IsAotCompatible` enables the + trim/AOT analyzers, so warnings fail the build. That is deliberate. +- `dotnet test` is green. Add tests for behaviour you changed. +- If you touched the console, run it and look at it — in Windows Terminal *and* in `conhost.exe`. + Several defects here were invisible in review and obvious on screen. +- If you touched anything under `docs/`, walk the doc-hygiene checklist in + [`CLAUDE.md`](CLAUDE.md#doc-hygiene-checklist-run-on-every-md-update). + +## Things that will get a PR sent back + +**Contract changes without discussion.** These four surfaces are normative because a browser and +Android port are planned and must behave identically: + +- `IChatProvider` and the records around it +- The `ChatErrorKind` taxonomy +- The `%APPDATA%\OpenKey\` file layout and JSON shapes +- Rotation rules + +Changing any of them means updating the contract doc first, then every implementation. Open an +issue before writing code. + +**A colour or glyph literal outside `Ui/Theme.cs` or `Ui/Glyphs.cs`.** The console was previously +styled at call sites and drifted into three different cases and four border styles. Everything +visible is a component in `Ui/Components.cs`. + +**An error message with no next step.** Every failure the user can see must say what happened and +what to do about it. A dead end is a bug, not a rough edge. + +**Internal vocabulary on screen.** No `DPAPI`, `OAuth`, `PKCE`, `429`, or `ChatErrorKind` on any +surface a user reads. HTTP status codes may appear only in a card's grey detail line. + +**Core depending on a host or a provider.** `OpenKey.Core` has no package references and no +knowledge of the console. Keep it that way. + +## Layout + +| Project | What it is | +|---|---| +| `src/OpenKey.Core` | Engine, rotation, storage, contracts. No dependencies. | +| `src/OpenKey.Providers.OpenRouter` | OpenRouter wire format and SSE. | +| `src/OpenKey` | Console host, UI, DPAPI key store, OAuth. | +| `tests/OpenKey.Core.Tests` | Engine, rotation, storage. | +| `tests/OpenKey.Tests` | Provider, console UI, PKCE. | + +Start with [`docs/architecture/`](docs/architecture/) — particularly +[`08-decisions.md`](docs/architecture/08-decisions.md), which records things that were tried or +considered and rejected, so they don't get re-proposed. + +Testing conventions are in [`docs/09-testing.md`](docs/09-testing.md). + +## Commits + +Conventional Commits (`feat:`, `fix:`, `docs:`, `test:`, `chore:`). Explain *why* in the body when +it isn't obvious from the diff — most of the valuable commit messages in this repo describe a +failure mode, not a code change. + +## Releases + +Tag `vX.Y.Z` and the release workflow builds and attaches both architectures. Release titles carry +the version only — no phase numbers on any public surface. diff --git a/Directory.Build.props b/Directory.Build.props index 76920d4..f4256df 100644 --- a/Directory.Build.props +++ b/Directory.Build.props @@ -8,7 +8,7 @@ true latest Recommended - 0.1.0 + 0.2.0 en-US diff --git a/LICENSE b/LICENSE new file mode 100644 index 0000000..46a2fcf --- /dev/null +++ b/LICENSE @@ -0,0 +1,21 @@ +MIT License + +Copyright (c) 2026 Paolo Patron + +Permission is hereby granted, free of charge, to any person obtaining a copy +of this software and associated documentation files (the "Software"), to deal +in the Software without restriction, including without limitation the rights +to use, copy, modify, merge, publish, distribute, sublicense, and/or sell +copies of the Software, and to permit persons to whom the Software is +furnished to do so, subject to the following conditions: + +The above copyright notice and this permission notice shall be included in all +copies or substantial portions of the Software. + +THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR +IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, +FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE +AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER +LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, +OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE +SOFTWARE. diff --git a/README.md b/README.md index fa5352c..3358fcf 100644 --- a/README.md +++ b/README.md @@ -1,51 +1,100 @@ # OpenKey -Click-and-play Windows console chat client for free OpenRouter LLMs. Double-click the `.exe`, paste an OpenRouter API key once, and chat. Auto-rotates across free models on rate-limits. Single self-contained binary — runs from a USB stick. +Chat with capable AI models for free, from a single Windows `.exe`. Nothing to install, no +subscription. Double-click, sign in once, and type. -## Run from source +When a free model is busy, OpenKey quietly moves to another one and you still get your answer. -```cmd -dotnet run --project src\OpenKey\OpenKey.csproj ``` +── OpenKey v0.1.0 ────────────────────────────────────────────────── +Developed by Paolo Patron -## Build single-file exe +┌─ Getting started ─────────────────────────────────────────────────┐ +│ Type a message and press Enter to chat. │ +│ │ +│ /models Choose which AI model answers you │ +│ /help See everything OpenKey can do │ +│ /quit Close OpenKey │ +└───────────────────────────────────────────────────────────────────┘ -```cmd -dotnet publish src\OpenKey\OpenKey.csproj -c Release -r win-x64 ^ - --self-contained true ^ - -p:PublishSingleFile=true ^ - -p:IncludeNativeLibrariesForSelfExtract=true ^ - -p:EnableCompressionInSingleFile=true ^ - -p:PublishReadyToRun=true ^ - -o publish\ +Patron ❯ explain server-sent events in one line + +OpenKey AI · deepseek/deepseek-chat-v3:free · 1.2s + +A one-way HTTP stream where the server pushes `data:` lines as they +happen, instead of the client polling for them. + +Patron ❯ ``` -Output: `publish\OpenKey.exe`. +## Get it + +Download `OpenKey.exe` from [Releases](https://github.com/corecompiled/OpenKey/releases) and +double-click it. It runs from a USB stick. + +You'll need a free [OpenRouter](https://openrouter.ai) key. OpenKey can fetch one through your +browser on first run, or you can paste one you already have. Either way it's encrypted for your +Windows account and stays on your PC. + +Full walkthrough: [`docs/08-user-guide.md`](docs/08-user-guide.md). ## Commands | Command | Effect | -|---------|--------| -| `/about` | Show version, data dir, active model, developer | -| `/models` | Pick a free model with arrow keys (pinned until restart) | -| `/model` | Show current active model | -| `/cls` | Clear the screen and reprint the header | -| `/help` | List all commands | -| `/reset` | Wipe `%APPDATA%\OpenKey\` and re-run first-run setup | -| `/quit` | Exit (alias: `/exit`) | +|---|---| +| `/new` | Start a fresh conversation, keeping your key | +| `/retry` | Send your last message again | +| `/history` | Show the conversation so far | +| `/copy` | Copy the last reply to the clipboard | +| `/export [path]` | Save the conversation as a markdown file | +| `/models` | Choose which AI model answers you | +| `/model` | Show which model is answering right now | +| `/theme` | Switch colours: default, dark, light, mono | +| `/about` | Version, where your data lives, who made it | +| `/cls` | Clear the screen | +| `/help` | List all commands | +| `/reset` | Erase everything and start over | +| `/quit` | Close OpenKey (alias: `/exit`) | -## Where data is stored +Anything not starting with `/` is sent to the AI. Ctrl+C stops a reply in progress. -`%APPDATA%\OpenKey\` (typically `C:\Users\\AppData\Roaming\OpenKey\`). The API key is DPAPI-encrypted per Windows user — only the user who entered it on this PC can decrypt. +## Your data -## Get an OpenRouter API key +Stored in `%APPDATA%\OpenKey\`. Your key is encrypted so only your Windows account on this PC can +read it; your conversation is plain JSON. OpenKey talks to OpenRouter and nowhere else, and +collects no telemetry of any kind. See [`SECURITY.md`](SECURITY.md). -Sign up free at . OpenKey uses only free-tier models. +## Build from source + +Requires the .NET SDK pinned in `global.json`. Windows only. + +```cmd +dotnet run --project src\OpenKey\OpenKey.csproj # run +dotnet test # 65 tests +dotnet publish src\OpenKey\OpenKey.csproj -c Release -r win-x64 -o publish\ +``` + +Publish settings live in the project file, so that last line produces the shipping binary — no +flags to remember. Details in [`CONTRIBUTING.md`](CONTRIBUTING.md). ## Docs -Full project docs in [`docs/`](docs/). Start with [`docs/00-overview.md`](docs/00-overview.md). Meta-rules for Claude sessions in [`CLAUDE.md`](CLAUDE.md). +| | | +|---|---| +| [User guide](docs/08-user-guide.md) | Every feature, and what to do when something breaks | +| [Architecture](docs/architecture/) | How it works, and [why it works that way](docs/architecture/08-decisions.md) | +| [Backlog](BACKLOG.md) | What's next, and what's deliberately not happening | +| [Changelog](CHANGELOG.md) | What changed | +| [Contributing](CONTRIBUTING.md) | Building, testing, house rules | + +Project overview: [`docs/00-overview.md`](docs/00-overview.md). ## Status -Desktop console exe available now. A browser (PWA) app and an Android app are planned. +The Windows desktop app works today. A browser app and an Android app reuse the same contracts and +are planned; a desktop GUI, tool use, and local document search come first. See +[`docs/07-roadmap.md`](docs/07-roadmap.md). + +## Licence + +MIT — see [`LICENSE`](LICENSE). diff --git a/SECURITY.md b/SECURITY.md new file mode 100644 index 0000000..506e1a2 --- /dev/null +++ b/SECURITY.md @@ -0,0 +1,40 @@ +# Security + +## Reporting a vulnerability + +Open a [private security advisory](https://github.com/corecompiled/OpenKey/security/advisories/new) +on the repository. Please don't file a public issue for anything exploitable. + +Include what you did, what happened, and what you expected. A proof of concept helps. + +## What OpenKey does with your data + +- **Your API key** is encrypted with Windows DPAPI under `DataProtectionScope.CurrentUser` and + written to `%APPDATA%\OpenKey\key.bin`. Only the same Windows account on the same machine can + decrypt it. Copying the file to another PC or another user account yields nothing usable. +- **Your conversations** are stored in plain JSON at `%APPDATA%\OpenKey\session.json`. They are not + encrypted. Anyone with access to your Windows account can read them. `/reset` deletes them. +- **Network traffic** goes to `openrouter.ai` and nowhere else. The only other connection OpenKey + ever opens is a local `http://localhost:3000/callback` listener, briefly, during browser sign-in. +- **No telemetry.** No analytics, no crash reporting, no phone-home, in any phase. This is a + standing project rule, not a current default. + +## Threat model + +OpenKey is a single-user desktop application. It assumes the Windows account it runs under is +trusted. It does not defend against: + +- Another process running as the same user (DPAPI cannot help here — that process can decrypt the + key exactly as OpenKey does). +- Physical access to an unlocked machine. +- A malicious OpenRouter endpoint, beyond ordinary TLS certificate validation. + +## Handling of model output + +Model replies are untrusted input and are treated as such. Every literal is escaped before it +reaches the console renderer, so a reply cannot inject console markup, forge UI chrome, or emit +control sequences. The fallback path taken when markdown parsing fails re-escapes as well. + +## Supported versions + +OpenKey is pre-1.0. Only the latest release receives fixes. diff --git a/docs/00-overview.md b/docs/00-overview.md index d2ae72f..3201b22 100644 --- a/docs/00-overview.md +++ b/docs/00-overview.md @@ -18,8 +18,8 @@ OpenKey is a click-and-play Windows console chat client for free LLMs available | Phase | Goal | |-------|------| -| **1** | Console chat, OpenRouter free models, rotation, persist + resume last session, `/reset`, guided first-run (OAuth/PKCE + paste) | -| **1.1** | QoL: `/history`, `/export`, `/help`, version banner, active-model indicator | +| **1** | Console chat, OpenRouter free models, rotation, persist + resume last session, `/reset`, guided first-run (OAuth/PKCE + paste), `/help`, `/about`, `/models`, version banner, streaming reply render | +| **1.1** | QoL: `/new`, `/stop`, `/retry`, `/history`, `/export` | | **1.2** | QoL: token counter, `/config` editor, update checker, theme toggle, multi-key support | | **2** | GUI (Avalonia), same Core/providers | | **3** | Tool use / function calling (file ops, web fetch, sandboxed) | @@ -44,12 +44,16 @@ OpenKey is a click-and-play Windows console chat client for free LLMs available - Both acquisition paths validate against OpenRouter `/models` before persisting. - After the key is saved, the screen clears and the banner reprints before the chat REPL opens. - REPL labels read `:` and `OpenKey AI:` (uses `Environment.UserName`). -- When a message is sent, a `OpenKey AI is thinking…` spinner shows until the reply completes, then the reply prints as rendered markdown (bold, italic, code, headings, lists). The engine still streams chunks internally; only the display is buffered so markdown renders cleanly with no per-token re-parse. +- When a message is sent, a `Thinking` spinner shows until the **first token**, then a + `OpenKey AI · · ` header appears and the reply streams in. Each markdown block is + repainted styled as it completes (bold, italic, code, headings, lists), while the still-arriving + tail stays plain — styled means settled, raw means still coming. Superseded the earlier + buffer-until-complete design, which showed nothing at all until the reply finished. - Banner shows `OpenKey vX.Y.Z` inline on the Spectre `Rule`, with subline `Developed by Paolo Patron`. Data dir is surfaced via `/about`, not the banner. - `/help` prints a Spectre table of all supported commands (`/about`, `/models`, `/model`, `/cls`, `/help`, `/reset`, `/quit`). - `/about` prints a Spectre panel with version, data dir, active model, pinned model, developer. - `/models` opens a Spectre `SelectionPrompt` over the free-model catalog (arrow keys, enter to select). Selection pins the model for subsequent requests until app restart; rotation still kicks in if the pinned model is rate-limited. -- On a forced rate-limit, the app silently rotates to the next free model and continues. +- On a forced rate-limit, the app rotates to the next free model and continues, noting it with a single quiet line ("Moved past 2 busy models."). No error, no per-attempt warnings. - Closing and re-launching shows the previous session restored. - `/reset` confirms, wipes `%APPDATA%\OpenKey\`, and re-runs first-run flow. diff --git a/docs/01-architecture.md b/docs/01-architecture.md index 0df6077..a9331b7 100644 --- a/docs/01-architecture.md +++ b/docs/01-architecture.md @@ -74,7 +74,7 @@ public sealed record ModelInfo( ``` C:\Users\Patron\OpenKey\ ├── docs\ ← this guide set -├── OpenKey.sln +├── OpenKey.slnx └── src\ ├── OpenKey\ ← Console app, entry point, hosts UI │ └── OpenKey.csproj (outputs OpenKey.exe) @@ -176,7 +176,7 @@ public sealed class ChatException : Exception ## Cross-UI contract -OpenKey will eventually ship on three surfaces: desktop console exe (Phase 1), desktop GUI (Phase 2), PWA (Phase 6), Android APK (Phase 7). The C# implementation here is the **reference impl**. Other impls are ports. +OpenKey will eventually ship on four surfaces: desktop console exe (Phase 1), desktop GUI (Phase 2), PWA (Phase 6), Android APK (Phase 7). The C# implementation here is the **reference impl**. Other impls are ports. To keep all surfaces aligned, the `docs/` directory is the **language-agnostic contract**. The following sections are **normative** — any UI / language implementation must match them exactly: diff --git a/docs/02-phase1-build.md b/docs/02-phase1-build.md index e114c49..8c265fb 100644 --- a/docs/02-phase1-build.md +++ b/docs/02-phase1-build.md @@ -14,7 +14,7 @@ End-to-end build steps. Follow top to bottom on a clean machine. From `C:\Users\Patron\OpenKey\`: ```cmd -dotnet new sln -n OpenKey +dotnet new sln -n OpenKey # the repo now uses the newer OpenKey.slnx format dotnet new console -n OpenKey -o src\OpenKey --framework net10.0 dotnet new classlib -n OpenKey.Core -o src\OpenKey.Core --framework net10.0 @@ -33,10 +33,19 @@ dotnet add src\OpenKey.Providers.OpenRouter\OpenKey.Providers.OpenRouter.csproj ```cmd dotnet add src\OpenKey\OpenKey.csproj package Spectre.Console +dotnet add src\OpenKey\OpenKey.csproj package Markdig dotnet add src\OpenKey\OpenKey.csproj package Microsoft.Extensions.DependencyInjection -dotnet add src\OpenKey\OpenKey.csproj package Microsoft.Extensions.Hosting +dotnet add src\OpenKey\OpenKey.csproj package System.Security.Cryptography.ProtectedData ``` +Two corrections to what this step originally said: + +- **Markdig is required.** `MarkdownConsoleRenderer` is built on it, so omitting it here produced a + project that does not compile. +- **`Microsoft.Extensions.Hosting` is not used.** A REPL needs no generic host, hosted-service + lifetime, or configuration binding; `Program.cs` composes its dozen services explicitly. See + [`architecture/08-decisions.md`](architecture/08-decisions.md). + `OpenKey.Core` and `OpenKey.Providers.OpenRouter` use only BCL (`System.Text.Json`, `System.Net.Http`, `System.Security.Cryptography.ProtectedData`). DPAPI lives in the `System.Security.Cryptography.ProtectedData` NuGet (it was removed from the SDK BCL on non-Windows targets): ```cmd @@ -45,24 +54,37 @@ dotnet add src\OpenKey.Core\OpenKey.Core.csproj package System.Security.Cryptogr ## Step 3 — Project file edits -`src\OpenKey\OpenKey.csproj` — set output, version, icon (optional): +Shared settings — `TargetFramework`, `Nullable`, `ImplicitUsings`, `Version`, +`TreatWarningsAsErrors` — live in **`Directory.Build.props`** at the repo root, not in each project. +Don't repeat them per project; they apply automatically. + +`src\OpenKey\OpenKey.csproj` carries only what is specific to the host: ```xml Exe - net10.0 + + net10.0-windows OpenKey OpenKey - 0.1.0 - enable - enable - win-x64 + win-x64;win-arm64 + false + + + true + true + true + true + true + true ``` -`OpenKey.Core.csproj` and `OpenKey.Providers.OpenRouter.csproj` — just enable nullable + implicit usings. +`OpenKey.Core.csproj` and `OpenKey.Providers.OpenRouter.csproj` need almost nothing beyond +`IsAotCompatible` and their `InternalsVisibleTo` entries. ## Step 4 — File-by-file scaffold @@ -234,12 +256,9 @@ public sealed class CommandRouter } ``` -`/reset` flow: -1. Spectre `Confirm("Wipe all OpenKey data and re-run setup?")` -2. `_keyStore.Clear(); _sessions.Clear(); _catalog.ClearCache(); _rotation.Clear();` -3. Delete `%APPDATA%\OpenKey\` directory -4. Call `EnsureFirstRunAsync()` again -5. Continue REPL +`/reset` semantics are normative in +[`05-persistence-and-reset.md`](05-persistence-and-reset.md#reset-semantics) — follow that, not a +copy here. Both files previously spelled out divergent sequences. ## First-run flow (sequence) @@ -256,7 +275,7 @@ keyStore.HasKey()? ── no ─→ Spectre SelectionPrompt: │ │ │ │ │ OpenRouterOAuth.AcquireKeyAsync(ct): │ │ 1. generate PKCE pair - │ │ 2. start HttpListener on 127.0.0.1:/callback + │ │ 2. start HttpListener on localhost:3000/callback (fixed) │ │ 3. Process.Start the openrouter.ai/auth URL │ │ 4. await callback ?code=... (timeout 5min) │ │ 5. POST /api/v1/auth/keys exchange → user_key @@ -283,34 +302,39 @@ catalog.GetFreeModelsAsync() (cache or refresh) ↓ engine.ResumeAsync() (loads session.json if present) ↓ -REPL (":" prompt; "OpenKey AI is thinking…" spinner until the reply completes, then "OpenKey AI:" header + markdown-rendered reply) +REPL (" ❯ " prompt; "Thinking" spinner until the FIRST token, then a + "OpenKey AI · · " header and a block-by-block streamed reply) ``` OAuth wire details live in `03-openrouter-integration.md` § "OAuth / PKCE". Storage behavior unchanged (DPAPI-encrypted `key.bin`) — OAuth is just a UX option for *obtaining* the key. ## Acceptance checklist (Phase 1 done) -- [ ] `dotnet run --project src\OpenKey` launches Spectre banner -- [ ] First-run menu offers (1) Sign in with browser (OAuth/PKCE), (2) Paste an existing key -- [ ] During OAuth wait, hint `[p] paste, [c] cancel` is visible -- [ ] Pressing `p` during OAuth wait drops to paste prompt within the same first-run attempt -- [ ] Pressing `c` (or Esc) during OAuth wait returns to the menu -- [ ] OAuth path: browser opens to `openrouter.ai/auth` with `callback_url=http://localhost:3000/callback` (fixed per docs), callback returns a key, app validates + persists DPAPI-encrypted -- [ ] If port 3000 is in use, OpenKey reports it and offers paste fallback in the same first-run attempt -- [ ] Paste path: secret prompt accepts a key, validates against `/models`, persists DPAPI-encrypted -- [ ] After key validation, screen is cleared and banner reprinted before the chat REPL opens -- [ ] REPL prompt label uses the current Windows username (`Environment.UserName`) followed by `: ` -- [ ] AI label reads `OpenKey AI:` (no `❯` glyph) -- [ ] Banner rule reads `OpenKey vX.Y.Z` (version inline); subline reads `Developed by Paolo Patron`. No data dir on the banner. -- [ ] `/help` prints a Spectre table listing `/about`, `/models`, `/model`, `/cls`, `/help`, `/reset`, `/quit` -- [ ] `/about` prints a Spectre panel with version, data dir, active model, pinned model, developer -- [ ] `/models` opens a Spectre `SelectionPrompt` over the free-model catalog with `↑/↓` navigation; selecting a model pins it (via `ChatEngine.PreferredModelId`) until restart; selecting "Auto" clears the pin -- [ ] Reply text is never truncated — the full buffered reply (including the final chunk) is rendered and the cursor returns to the prompt without a perceived hang -- [ ] When a message is sent, a `OpenKey AI is thinking…` spinner shows until the reply completes; then the markdown-rendered reply prints (bold, italic, inline/fenced code, headings, lists) -- [ ] Killing model 1 (e.g., set temporary `cooldownUntil` via test hook) auto-rotates to model 2 mid-conversation -- [ ] `/model` prints `current model: ` -- [ ] `/reset` confirms, wipes, re-runs the first-run menu in same process -- [ ] `/quit` exits cleanly -- [ ] Close + reopen → previous session restored, last 2 turns shown -- [ ] Ctrl+C while the reply spinner is active cancels cleanly, returns to prompt -- [ ] Build via the `dotnet publish` command in `06-build-and-distribute.md` produces a runnable single `.exe` +All met as of the production-readiness pass. Several items were reworded when the console was +rebuilt — the originals described a buffered spinner and an `OpenKey AI:` label that no longer +exist. Automated coverage is in `tests/`; see [`09-testing.md`](09-testing.md). + +- [x] `dotnet run --project src\OpenKey` launches the banner +- [x] First-run menu offers (1) Sign in with browser, (2) Paste an existing key +- [x] During OAuth wait, the hint to press `P` to paste or `Esc` to cancel is visible +- [x] Pressing `P` during OAuth wait drops to the paste prompt within the same attempt +- [x] Pressing `Esc` during OAuth wait returns to the menu +- [x] OAuth path: browser opens to `openrouter.ai/auth` with `callback_url=http://localhost:3000/callback` (fixed), callback returns a key, app validates and persists it DPAPI-encrypted +- [x] If port 3000 is in use, OpenKey says so and offers paste in the same attempt +- [x] Paste path: secret prompt accepts a key, validates against `/models`, persists it +- [x] After validation the screen clears and the banner reprints before the REPL opens +- [x] REPL prompt is ` ❯ `, degrading to `>` where the glyph is unsafe +- [x] Reply header is `OpenKey AI · · `, printed before the first token arrives +- [x] Banner reads `OpenKey vX.Y.Z` with subline `Developed by Paolo Patron`; no data dir on the banner +- [x] `/help` lists `/models`, `/model`, `/about`, `/cls`, `/help`, `/reset`, `/quit` +- [x] `/about` shows version, active model, model choice, data dir, key handling, developer +- [x] `/models` opens a picker showing display name and context size; selecting one pins it until restart; "Auto" clears the pin +- [x] Reply text is never truncated; the cursor returns to the prompt without a perceived hang +- [x] A `Thinking` spinner shows until the **first token**, then the reply streams block by block with bold, italic, inline and fenced code, headings and lists rendered +- [x] A forced failure on model 1 rotates to model 2 mid-conversation, and the reply appears **once** (covered by `ChatEngineTests`) +- [x] `/model` names the current model +- [x] `/reset` states what it erases, confirms, wipes, and re-runs setup in the same process +- [x] `/quit` exits cleanly, holding the window when double-clicked +- [x] Close and reopen restores the previous session and shows the last 2 turns +- [x] Ctrl+C during a reply cancels that reply only; **the next message still works** +- [x] `dotnet publish -c Release -r win-x64` produces a runnable single `.exe` with no extra flags diff --git a/docs/03-openrouter-integration.md b/docs/03-openrouter-integration.md index 3df9da7..6849825 100644 --- a/docs/03-openrouter-integration.md +++ b/docs/03-openrouter-integration.md @@ -12,7 +12,7 @@ Wire-level details for `OpenRouterProvider : IChatProvider`. Authoritative exter |--------|-------|-------| | `Authorization` | `Bearer ` | Loaded from `IKeyStore` | | `Content-Type` | `application/json` | POSTs only | -| `HTTP-Referer` | `https://github.com//openkey` or `https://openkey.local` | OpenRouter uses this for app analytics; can be any URL we own/control. Use a fixed app constant. | +| `HTTP-Referer` | `https://github.com/corecompiled/OpenKey` or `https://openkey.local` | OpenRouter uses this for app analytics; can be any URL we own/control. Use a fixed app constant. | | `X-Title` | `OpenKey` | Shown in OpenRouter dashboard | Define once in `OpenRouterProvider`: @@ -255,7 +255,7 @@ Response body: Persist `key` exactly like a pasted key — DPAPI on desktop, platform-equivalent on PWA/Android (see `05-persistence-and-reset.md`). -### Error mapping +### OAuth error mapping | HTTP | Kind | Notes | |------|------|-------| diff --git a/docs/04-model-rotation.md b/docs/04-model-rotation.md index 1f4f7f7..14a8f5d 100644 --- a/docs/04-model-rotation.md +++ b/docs/04-model-rotation.md @@ -73,7 +73,7 @@ Base cooldowns by `ChatErrorKind`: ### Exponential backoff -For repeated failures (`FailureCount` increments), multiply base by `2^(FailureCount-1)`, capped at **5 minutes**: +For repeated failures (`FailureCount` increments), multiply base by `2^FailureCount`, capped at **5 minutes**. The cap dominates quickly: a nominal 1 hour cooldown is clamped to 5 minutes like everything else. ```csharp var multiplier = Math.Min(1 << Math.Min(state.FailureCount, 8), 64); @@ -96,7 +96,8 @@ for (int attempt = 1; attempt <= MaxAttempts; attempt++) var model = await _rotation.PickAsync(candidates, ct); ActiveModel = model; - AnsiConsole.MarkupLine($"[grey](trying {model.Id})[/]"); + // NOTE: Core never writes to the console. Rotation is reported by the host, once, after the + // fact — see "User feedback" below. try { @@ -110,7 +111,7 @@ for (int attempt = 1; attempt <= MaxAttempts; attempt++) catch (ChatException ex) when (IsTransient(ex.Kind)) { _rotation.MarkFailure(model.Id, ex.Kind, ex.RetryAfterHint); - AnsiConsole.MarkupLine($"[yellow]rotating: {model.Id} → {ex.Kind}[/]"); + OnRotation?.Invoke($"{model.Id} → {ex.Kind}"); // host counts these; it does not print them continue; } catch (ChatException ex) @@ -136,7 +137,9 @@ static bool IsTransient(ChatErrorKind k) => If `StreamChatAsync` yields some chunks then throws: 1. Do **not** yield the partial assistant content as a final turn. -2. Clear any rendered partial output from the console (Spectre `AnsiConsole.MarkupLine` a newline + status). +2. Emit a `ChatChunk` with `IsAttemptRestart` set **before** any text from the next attempt. Core + does not touch the console; the consumer clears what it has drawn. Without this signal a + mid-reply rotation renders the answer twice concatenated while the saved session stores it once. 3. Re-enter the retry loop with the *same* user-turn history (assistant turn not yet appended). 4. The next model gets a fresh start with the same prompt. @@ -172,12 +175,26 @@ Phase 1.2 can swap in a real tokenizer (`Tiktoken`-equivalent) without changing ## User feedback -Spectre messages emitted by the rotation flow: +Rotation emits **no output of its own**. `ChatEngine` raises `OnRotation` and the host decides what, +if anything, to show. -- `[grey](trying meta-llama/llama-3.3-70b-instruct:free)[/]` — on each attempt -- `[yellow]rotating: → TransientRateLimit[/]` — on retryable failure -- `[red]all free models exhausted — try again in a few minutes[/]` — after `MaxAttempts` -- `[red]auth failure — run /reset to re-enter your API key[/]` — on `AuthFailure` +The console shows a single grey line above the reply, and only when rotation actually occurred: + +``` +Moved past 2 busy models. +``` + +Three rules, all learned the hard way: + +- **Not a warning.** Rotation working correctly is the feature doing its job. Yellow per-attempt + lines made a healthy app look broken. +- **Not during the reply.** Writing mid-stream corrupts the spinner and the streamed text, because + both own the cursor. +- **No model ids and no `ChatErrorKind` names.** The model that answered is already in the reply + header; the ones that didn't are not the user's problem. + +Failures that stop the turn are rendered as error cards by the host — see +[`architecture/07-error-taxonomy.md`](architecture/07-error-taxonomy.md) for the copy per kind. ## Persistence of rotation state diff --git a/docs/05-persistence-and-reset.md b/docs/05-persistence-and-reset.md index d930571..159e4b8 100644 --- a/docs/05-persistence-and-reset.md +++ b/docs/05-persistence-and-reset.md @@ -136,7 +136,16 @@ See `04-model-rotation.md` § "Persistence of rotation state". } ``` -If absent, defaults apply (empty list ⇒ use catalog-derived order). Phase 1 doesn't expose a UI to edit this — user edits the file manually. Phase 1.2 adds `/config`. +If absent, defaults apply (empty list means rotation chooses freely). + +Owned by `IConfigStore` / `JsonConfigStore`. `preferredModels` is how a pinned model is expressed — +a single entry — so `/models` now survives a restart. `/theme` writes `theme`. + +The file is meant to be hand-editable, so every field is treated as untrusted on load: blank model +ids are dropped, `theme` is lower-cased and falls back to `default` if unknown, and `maxTokens` +outside a sane range reverts to 2048. A corrupt file is renamed to `config.json.broken-` and +defaults apply, exactly as with a corrupt session — preferences are never worth failing a launch +over. ## First-run flow (detailed) diff --git a/docs/06-build-and-distribute.md b/docs/06-build-and-distribute.md index 6995ace..ea65e41 100644 --- a/docs/06-build-and-distribute.md +++ b/docs/06-build-and-distribute.md @@ -7,15 +7,19 @@ Producing the click-and-play `OpenKey.exe`. From `C:\Users\Patron\OpenKey\`: ```cmd -dotnet publish src\OpenKey\OpenKey.csproj ^ - -c Release ^ - -r win-x64 ^ - --self-contained true ^ - -p:PublishSingleFile=true ^ - -p:IncludeNativeLibrariesForSelfExtract=true ^ - -p:EnableCompressionInSingleFile=true ^ - -p:PublishReadyToRun=true ^ - -o publish\ +dotnet publish src\OpenKey\OpenKey.csproj -c Release -r win-x64 -o publish\ +``` + +**Every publish flag lives in `OpenKey.csproj`.** Don't pass them here and don't document a +different command anywhere else. They used to exist only as prose in the README, which meant any +publish that didn't paste that exact line — including CI — silently produced a *different artifact* +than the one that had been smoke-tested. Changing how the binary is built is a project-file edit, +reviewed like any other. + +For Windows on ARM, swap the RID: + +```cmd +dotnet publish src\OpenKey\OpenKey.csproj -c Release -r win-arm64 -o publish\ ``` ## Release naming convention @@ -23,7 +27,7 @@ dotnet publish src\OpenKey\OpenKey.csproj ^ Match the house style used across CoreCompiled repos (e.g. [SnagLite](https://github.com/corecompiled/SnagLite/releases)): - **Release title = the tag, version only** — `v0.1.0`. No app name, no "Phase X", no tagline in the title. -- **Body structure**: one-line pitch → `## What's in this release` (bullet the downloadable assets) → `## Quick start` → a `Full docs: [README](…)` link. +- **Body structure**: one-line pitch → `## What's in this release` (bullet the downloadable assets) → `## Quick start` → a `Full docs: [README](https://github.com/corecompiled/OpenKey#readme)` link. - **Never** reference internal phase numbering in any public surface (release page, commit messages, README). Output: `C:\Users\Patron\OpenKey\publish\OpenKey.exe` @@ -32,14 +36,18 @@ That single file is the entire deliverable. Copy it to a USB stick, give it to a ## Flag rationale -| Flag | Why | +All set in `OpenKey.csproj`, not on the command line. + +| Setting | Why | |------|-----| -| `--self-contained true` | Bundles the .NET runtime into the exe. User doesn't need .NET installed. | -| `-p:PublishSingleFile=true` | One file output (vs a folder of DLLs). | -| `-p:IncludeNativeLibrariesForSelfExtract=true` | Native libs (e.g., DPAPI shim) bundled and extracted to a temp dir at runtime. Required for a true single-file experience. | -| `-p:EnableCompressionInSingleFile=true` | Cuts ~30% off file size. Trade: slightly slower first launch (one-time decompression). | -| `-p:PublishReadyToRun=true` | Pre-jits IL for faster startup. Larger exe but the cmd app feels instant. | -| `-r win-x64` | Single RID. Phase 1 is Windows only. | +| `SelfContained` | Bundles the .NET runtime. The user doesn't need .NET installed — this is what makes it click-and-play. | +| `PublishSingleFile` | One file instead of a folder of DLLs. | +| `IncludeNativeLibrariesForSelfExtract` | Native libraries bundled and extracted to a temp dir at runtime. Required for a genuine single file. | +| `EnableCompressionInSingleFile` | Roughly 30% smaller, at the cost of a one-time decompression on first launch. | +| `PublishReadyToRun` | Pre-jits IL so startup feels instant. Larger file. | +| `IsAotCompatible` | Turns the trim/AOT analyzers on. Doesn't change the output; keeps the option open by failing the build on new reflection. | +| `RuntimeIdentifiers` | `win-x64;win-arm64`. | +| `InvariantGlobalization=false` | LLM replies are full of non-ASCII text. Costs ICU in the bundle; a deliberate trade. | ## Why NOT trimming @@ -48,22 +56,39 @@ That single file is the entire deliverable. Copy it to a USB stick, give it to a -p:PublishTrimmed=true ``` -Spectre.Console uses reflection internally (for prompt validators, table cells, etc.). Trimming will silently break the UI or crash with `MissingMethodException` at runtime. If we later want a smaller exe, we'd need to add `[DynamicallyAccessedMembers]` attributes throughout — not worth it for Phase 1. +Trimming is not enabled yet. The trim analyzers *are* on (`IsAotCompatible` is set on all three +projects) and the tree builds warning-clean, so the historical objection — that Spectre.Console's +internal reflection would silently break the UI — no longer applies unexamined. Enabling +`PublishTrimmed` now needs measurement rather than argument. -## Why NOT AOT +## Why NOT AOT (yet) ``` -# DO NOT add in Phase 1: +# Not enabled today: -p:PublishAot=true ``` -AOT (NativeAOT) produces a ~10MB exe with no JIT overhead. But: -- Spectre.Console v0.49+ has partial AOT support but quirks remain. -- DPAPI via `System.Security.Cryptography.ProtectedData` works under AOT, fine. -- `System.Text.Json` source generators are required (no reflection-based serialization). -- DI containers like `Microsoft.Extensions.DependencyInjection` work but constructor injection via reflection requires care. +**The reason originally given here has expired.** This section used to say Spectre.Console's +reflection blocked AOT. Measured from the shipped assemblies: `IsTrimmable` metadata is **absent** +in Spectre.Console 0.49.1 and **present** in 0.55.2. The library did the work. + +The other stated blocker, reflection-based `System.Text.Json`, is also gone — everything persisted +and every request body now goes through source-generated contexts. + +Current status: -Net: doable, but adds yank to the Phase 1 timeline. Defer to Phase 1.2 or Phase 2. +| Concern | State | +|---|---| +| Spectre.Console | Annotated trim/AOT-compatible since 0.55 | +| `System.Text.Json` | Source-generated contexts in place | +| Markdig | No analyzer warnings at our call sites | +| DPAPI via `ProtectedData` | AOT-safe | +| `Microsoft.Extensions.DependencyInjection` | Fine — composition is explicit, no assembly scanning | + +So what remains is measurement, not a known obstacle. The prize is real for a USB-distributed app: +roughly 42 MB → 15–20 MB, no extract-to-temp on first run, and faster startup. Tracked in +[`../BACKLOG.md`](../BACKLOG.md); rationale in +[`architecture/08-decisions.md`](architecture/08-decisions.md). ## Expected output @@ -76,7 +101,7 @@ Net: doable, but adds yank to the Phase 1 timeline. Defer to Phase 1.2 or Phase ## Versioning -Edit `src\OpenKey\OpenKey.csproj`: +Edit `Directory.Build.props` at the repo root — version is shared by every project: ```xml 0.1.0 @@ -93,38 +118,61 @@ AnsiConsole.MarkupLine($"[bold cyan]OpenKey[/] [grey]v{ver}[/]"); ``` SemVer: -- `1.0.0` — Phase 1 ship +- `1.0.0` — reserved for the first stable release; shipping versions so far are `0.x` - `1.1.0` — Phase 1.1 QoL - `1.2.0` — Phase 1.2 QoL - `2.0.0` — GUI (Phase 2) ## Smoke test checklist (manual, run after every publish) -Run the published `OpenKey.exe`: - -1. [ ] Launch shows banner with version -2. [ ] First-run shows a menu: (1) Sign in with browser, (2) Paste an existing key -3. [ ] During OAuth wait, hint `[p] paste, [c] cancel` is visible -4. [ ] Pressing `p` during OAuth wait drops to paste prompt within the same attempt -5. [ ] Pressing `c` (or Esc) during OAuth wait returns to the menu -6. [ ] OAuth path: browser opens to openrouter.ai/auth, user signs in, success page renders, console reports "✓ key validated and saved." -7. [ ] Paste path: invalid key shows red error and re-prompts (test once); valid key → "✓ key validated and saved." -8. [ ] After key validation, screen is cleared and banner reprinted before the chat REPL opens -9. [ ] REPL prompt label uses current Windows username + `: ` (e.g., `Patron: `); AI label reads `OpenKey AI:` -9a.[ ] Banner grey line shows `Developed by Paolo Patron` (non-italic) -9b.[ ] `/help` prints a Spectre table listing all supported commands -10. [ ] Callback listener binds to `http://localhost:3000/callback`. If Windows Defender Firewall prompts on first run, allow only "Private networks" -10a.[ ] If port 3000 is in use by another app, OpenKey reports it and offers paste fallback in the same attempt -11. [ ] Send "hello" → `OpenKey AI is thinking…` spinner shows until the reply completes, then the reply prints as rendered markdown (bold, code, headings, lists) -12. [ ] `/model` prints the active model id -13. [ ] Ctrl+C while the reply spinner is active cancels cleanly, returns to prompt -14. [ ] Close + relaunch → "Resumed session" + last 2 turns shown -15. [ ] `/reset` confirms, wipes, re-runs first-run menu in-process; on success the screen clears again before the chat REPL -16. [ ] After reset, send a message → works with newly-saved key -17. [ ] Forced rotation: temporarily edit `rotation.state.json` to set `cooldownUntil` far future for model #1 → next send picks model #2 -18. [ ] `/quit` exits cleanly with code 0 - -Document any deviation in `docs\smoke-results.md` (create if needed). +`dotnet test` covers the engine, provider, storage and streaming logic. This list is only for what +automation can't reach: a real terminal, a real browser, and a real double-click. + +**Setup** + +1. [ ] Launch shows the banner with a clean version (no `+` suffix) +2. [ ] First run offers: sign in with browser, or paste an existing key +3. [ ] During the browser wait, the hint to press `P` to paste or `Esc` to cancel is visible +4. [ ] Pressing `P` drops to the paste prompt within the same attempt +5. [ ] Pressing `Esc` returns to the menu +6. [ ] Browser path completes and the console reports the key was saved +7. [ ] Paste path: an invalid key shows a card explaining what to do, and re-prompts +8. [ ] If Windows Defender Firewall prompts, allowing "Private networks" only is sufficient +9. [ ] With port 3000 held by another app, OpenKey says so and offers paste in the same attempt + +**Chatting** + +10. [ ] Send "hello" → a `Thinking` spinner until the **first token**, then a + `OpenKey AI · · ` header and text streaming in +11. [ ] Ask for a reply containing a code fence and a list → the fence renders as a bordered panel, + the list renders with bullets, with one blank line between blocks +12. [ ] **A reply longer than the window scrolls normally and never erases earlier conversation** +13. [ ] Ctrl+C mid-reply stops that reply — **and the next message still works** +14. [ ] Resize the terminal mid-reply → output may lose styling on the block in flight, but earlier + conversation is untouched +15. [ ] Paste a multi-line snippet containing a line starting with `/` → sent as one message, and + the `/` line does **not** execute + +**Commands and state** + +16. [ ] `/help`, `/about`, `/model`, `/models`, `/cls` all render correctly +17. [ ] `/models` shows model names and context sizes, not raw ids +18. [ ] Close and relaunch → the last turns are shown in grey and the conversation continues +19. [ ] `/reset` states that it erases the conversation as well as the key, defaults to no, and + re-runs setup in-process +20. [ ] Forced rotation: set a far-future `cooldownUntil` for the first model in + `rotation.state.json` → the next message uses another model, shows one grey + "Moved past N busy models." line, and the reply appears **once** +21. [ ] `/quit` exits cleanly + +**Environment** + +22. [ ] Run in **legacy `conhost.exe`** as well as Windows Terminal → no glyph renders as a box, + including the spinner +23. [ ] `OpenKey.exe > out.txt` → plain text, no escape sequences, and the app does **not** exit + immediately +24. [ ] **Double-click the exe** → on `/quit` and on any fatal error the window waits for a keypress + instead of vanishing ## Antivirus / SmartScreen notes @@ -146,15 +194,19 @@ For Phase 1, ship unsigned + a one-line note in README that SmartScreen may warn 3. Copy `publish\OpenKey.exe` to USB drive root (or any folder) 4. Plug into a second Windows 10/11 machine 5. Run `OpenKey.exe` directly from USB -6. First-run prompt appears (because per-user provisioning — see 05-persistence-and-reset) +6. First-run prompt appears — state is per Windows user, so a second machine starts fresh 7. User pastes *their own* OpenRouter key → saved to *their* `%APPDATA%` on *that* machine 8. Verify chat works end-to-end -Note: the USB drive itself stores nothing user-specific. All state lives in `%APPDATA%` of whichever Windows user runs the exe. This is intentional (and aligns with the "Per-user provisioning" decision). +Note: the USB drive itself stores nothing user-specific. All state lives in `%APPDATA%` of whichever Windows user runs the exe. This is intentional: see [`05-persistence-and-reset.md`](05-persistence-and-reset.md). + +## CI -## CI build (optional, Phase 1.1) +Implemented — see `.github/workflows/ci.yml` (build, test, and a publish check on every push) +and `.github/workflows/release.yml` (both architectures attached to a `v*` tag). The sketch that +used to live here has been replaced by the real thing. -GitHub Actions workflow sketch: +For reference, the shape is: ```yaml name: build diff --git a/docs/07-roadmap.md b/docs/07-roadmap.md index 9da2487..1af724e 100644 --- a/docs/07-roadmap.md +++ b/docs/07-roadmap.md @@ -33,10 +33,14 @@ Small additive features that don't change the architecture. Hello! ... ``` - **`/new`** — clear in-memory turns (keep config and key). Effectively `_engine.NewSessionAsync()`. -- **`/about`** — Spectre panel with version, data dir, active model, pinned model, developer. *(shipped)* -- **`/models`** — Spectre `SelectionPrompt` arrow-key picker over the free-model catalog. Selection pins via `ChatEngine.PreferredModelId` until restart; rotation policy still owns cooldown / fallback. *(shipped)* -- **Banner** — `OpenKey vX.Y.Z` inline on the Spectre `Rule`, subline `Developed by Paolo Patron`. Data dir moved to `/about`. *(shipped)* -- **Active-model indicator** — show `(model-id)` prefix on every `ai ❯` line (already in `02-phase1-build.md`, lock it in here). +- **`/stop`** — cancel a running reply without Ctrl+C. Ctrl+C already works; nothing on screen says so. +- **`/retry`** — resend the last message, typically after a rotation or an error card. + +`/about`, `/models`, the version banner and the active-model indicator shipped in Phase 1 and are +documented in [`02-phase1-build.md`](02-phase1-build.md#acceptance-checklist-phase-1-done). They +were listed here by mistake: the tier ladder puts them at Tier 1, and the lowest applicable tier +wins. The active-model indicator landed as the `OpenKey AI · · ` reply header +rather than a per-line prefix. ## Phase 1.2 — QoL deeper @@ -44,7 +48,7 @@ Features that touch storage or DI but stay backward-compatible. - **Token counter** — estimate tokens per turn and show running total in status line. Use a real tokenizer NuGet (e.g., `Tiktoken` for OpenAI-family models, `MicrosoftDeepDev.Tokenizer` for cross-model). - **`/config`** — interactive Spectre menu to edit `config.json` (preferred model order, max_tokens, theme). -- **Update checker** — on launch, query `https://api.github.com/repos//openkey/releases/latest`. If newer, Spectre yellow notice with download URL. **Do not auto-download.** +- **Update checker** — on launch, query `https://api.github.com/repos/corecompiled/OpenKey/releases/latest`. If newer, Spectre yellow notice with download URL. **Do not auto-download.** - **Theme toggle** — `/theme dark|light|mono`. Stored in `config.json`. Spectre styles parameterized. - **Multi-key support**: - `/key add ` — add another OpenRouter key under a label @@ -179,8 +183,13 @@ public sealed class ClaudeCodeProvider : IChatProvider public Task> ListModelsAsync(CancellationToken ct) => Task.FromResult>(new[] { - new ModelInfo("claude-opus-4-7", "Claude Opus 4.7", 200_000, IsFree: true), - new ModelInfo("claude-sonnet-4-6", "Claude Sonnet 4.6", 200_000, IsFree: true), + // NOTE: IsFree here would mean "no incremental cost to this user", which is NOT what + // the flag means elsewhere — IModelCatalog filters on it to build the free-model list, + // so setting it true would surface paid models in the /models picker and violate the + // no-paid-models rule. Phase 5 needs a separate notion of "already paid for", not a + // reuse of IsFree. Resolve before implementing. + new ModelInfo("claude-opus-4-7", "Claude Opus 4.7", 200_000, IsFree: false), + new ModelInfo("claude-sonnet-4-6", "Claude Sonnet 4.6", 200_000, IsFree: false), }); public async IAsyncEnumerable StreamChatAsync(ChatRequest req, @@ -305,8 +314,17 @@ src/OpenKey.Pwa/ Flat list of small, non-blocking improvements. Pick off opportunistically. Items here are explicitly **not** on the tier ladder — they're standalone polish that doesn't sequence-block anything else. -- (seed) Richer banner ASCII art with version + active model -- (seed) Color-blind-friendly default Spectre theme +Both seed items are resolved — see [`../BACKLOG.md`](../BACKLOG.md) for detail. + +- ~~Colour-blind-friendly palette check~~ — done. Colour is never the only signal, and the `mono` + palette is the standing test: a unit test asserts no hue survives it, so anything that starts + depending on colour alone fails the build. +- ~~Richer banner artwork~~ — **deliberately not done.** The console design principle is that calm + beats decorative, and the banner is the first thing a non-technical user sees. Art would + contradict the design it is meant to serve. + +**Execution order and current state live in [`../BACKLOG.md`](../BACKLOG.md).** This file says what +and why; the backlog says what state each item is in and what happens next. Add new entries here only when they truly don't belong in a phase. If an idea even *might* fit a tier, put it in the tier. diff --git a/docs/08-user-guide.md b/docs/08-user-guide.md new file mode 100644 index 0000000..4f7a5e7 --- /dev/null +++ b/docs/08-user-guide.md @@ -0,0 +1,160 @@ +# 08 — User guide + +Everything OpenKey does, from a user's point of view. + +## Getting started + +Download `OpenKey.exe` and double-click it. Nothing to install, no runtime to add. It runs happily +from a USB stick. + +Windows may show a SmartScreen warning the first time, because the file isn't code-signed yet. +Choose **More info → Run anyway** if you trust where you got it from. + +### Signing in, once + +OpenKey needs a free OpenRouter key. Two ways: + +**Sign in with your browser** (recommended). OpenKey opens OpenRouter, you approve, and it collects +the key automatically. Press P at any point to switch to pasting instead, or +Esc to cancel. + +**Paste a key you already have.** Create one at . It won't appear on +screen as you type. + +Either way OpenKey checks the key works before saving it, and encrypts it for your Windows account. + +## Chatting + +Type and press Enter. Anything that doesn't start with `/` goes to the AI. + +While a reply arrives you'll see the model's name and how long it's taken. Text appears as it's +generated; formatting settles a paragraph at a time, so finished parts look tidy while the rest is +still coming. + +Press Ctrl+C to stop a reply you don't want. That cancels only that reply — +you can carry straight on. At the prompt with nothing running, Ctrl+C closes +OpenKey. + +Arrow keys, Home, End and F7 (recent lines) all work while typing. +Pasting several lines at once sends them as a single message. + +Your conversation is remembered. Next time you open OpenKey it shows the last couple of turns and +picks up where you left off. + +## Commands + +| Command | What it does | +|---|---| +| `/new` | Start a fresh conversation, keeping your key | +| `/retry` | Send your last message again | +| `/history` | Show the conversation so far | +| `/copy` | Copy the last reply to the clipboard | +| `/export [path]` | Save the conversation as a markdown file | +| `/models` | Choose which AI model answers you | +| `/model` | Show which model is answering right now | +| `/theme` | Switch colours: default, dark, light, mono | +| `/about` | Version, where your data lives, who made it | +| `/cls` | Clear the screen | +| `/help` | List these commands | +| `/reset` | Erase everything and start over | +| `/quit` | Close OpenKey (also `/exit`) | + +### `/new` vs `/reset` + +`/new` clears the conversation and keeps you signed in. That's what you want most of the time. + +`/reset` erases *everything*, including your key, and makes you sign in again. Use it only when +your key stopped working or you want to switch accounts. + +### `/export` + +`/export` on its own saves to your Desktop with a timestamped name. Give it a path to choose where: + +``` +/export C:\Users\Me\Documents\notes.md +``` + +### `/theme` + +`default` and `dark` suit a dark terminal, `light` suits a white background, and `mono` drops +colour entirely — useful for screenshots, high-contrast setups, or if colour is hard to +distinguish. Your choice is remembered. `/theme` on its own shows the current one. + +### `/models` + +Lists every free model, with its context size — roughly how much conversation it can hold at once. +Arrow keys to move, Enter to choose. + +Picking one sticks — including next time you open OpenKey. Choose **Auto** to hand the choice back +to OpenKey, which is the default and usually what you want. + +## Why the model sometimes changes + +Free models are shared and frequently busy. When one refuses, OpenKey quietly moves to another and +carries on — you just get your answer. If that happened you'll see a small note like *"Moved past 2 +busy models."* + +This is normal and is the main thing OpenKey does for you. If **every** free model is busy at once, +it says so and suggests waiting a moment. + +## Where your data lives + +`%APPDATA%\OpenKey\` — usually `C:\Users\\AppData\Roaming\OpenKey`. `/about` shows the exact +path. + +| File | What it is | +|---|---| +| `key.bin` | Your key, encrypted for your Windows account | +| `session.json` | Your conversation | +| `models.cache.json` | The model list, refreshed daily | +| `config.json` | Your theme and model preferences | +| `rotation.state.json` | Which models are busy | + +Your key is encrypted so that only your Windows account on this PC can read it — copying the file +to another machine gets someone nothing. **Your conversation is not encrypted**, so anyone who can +use your Windows account can read it. + +OpenKey talks to OpenRouter and nowhere else. No analytics, no tracking, ever. + +## When something goes wrong + +Every error says what happened and what to do next. The common ones: + +**"Your key was refused."** The key was revoked or replaced. `/reset` and sign in again. + +**"This key is out of credit."** Your free allowance is used up. Wait for it to renew, or add +credit at OpenRouter. + +**"Can't reach OpenRouter."** Usually your connection. If you're on hotel, airport or café Wi-Fi, +you probably still need to sign in to the network itself in a browser — OpenKey will tell you when +it detects that. + +**"Every free model is busy right now."** Wait a minute and resend, or use `/models` to pick one +directly. + +**"Browser sign-in isn't available right now."** OpenKey needs one of a few local ports for a +moment to receive the sign-in, and all of them are in use. Paste a key instead, or close whatever is +holding them. + +**Boxes or question marks instead of symbols.** You're in the older console. OpenKey normally +detects this and uses plain characters; if it slips through, run it from Windows Terminal. + +**The window closes instantly.** It shouldn't — OpenKey waits for a keypress before closing when +you've double-clicked it. If it still happens, run it from a terminal to see the message. + +## Questions + +**Does it cost anything?** No. OpenKey only ever uses free models. + +**Do I need an internet connection?** Yes, to reach OpenRouter. + +**Can I use it on another PC?** Yes, but you'll sign in again — the saved key is deliberately tied +to one Windows account on one machine. + +**Can I run it from a USB stick?** Yes. That's the intended way. Your data still goes to +`%APPDATA%` on whichever PC you use. + +**Is my conversation sent anywhere?** Only to OpenRouter, to generate replies. Nowhere else. + +**How do I start a fresh conversation without losing my key?** `/new`. `/reset` is the +all-or-nothing one. diff --git a/docs/09-testing.md b/docs/09-testing.md new file mode 100644 index 0000000..be10a53 --- /dev/null +++ b/docs/09-testing.md @@ -0,0 +1,94 @@ +# 09 — Testing + +```cmd +dotnet test +``` + +65 tests across two projects. xunit, no other test dependencies. + +| Project | Covers | Target | +|---|---|---| +| `tests/OpenKey.Core.Tests` | Engine, rotation, storage | `net10.0` | +| `tests/OpenKey.Tests` | Provider, console UI, PKCE | `net10.0-windows` | + +The split follows the platform boundary. Core's tests run wherever Core does; anything touching +DPAPI, the console, or Windows-only behaviour belongs in the second project. + +Both reach `internal` members through `InternalsVisibleTo`, declared in the corresponding +`.csproj`. + +## The two helpers everything is built on + +**`FakeChatProvider`** makes attempts scriptable: + +```csharp +provider.ThenFails(ChatErrorKind.TransientRateLimit) + .ThenSucceeds("recovered"); +``` + +Its absence is why `ChatEngine` — the entire retry and rotation state machine — had no coverage at +all before. It also records which models were called, which is how rotation order is asserted. + +**`TempAppPaths`** implements `IAppPaths` over a temp directory and deletes it on dispose. Because +`IAppPaths` derives every filename from `RootDir` via default interface members, a substitute needs +exactly one property — so storage tests touch a real filesystem instead of mocking one. + +## What is covered + +**Engine** — success and persistence; rotation on transient failure; the attempt-restart signal; +`NetworkDown` and `InvalidRequest` not rotating; user-turn removal on failure and on cancellation; +empty catalog; pinned-model preference; and streams ending without a final chunk. + +**Provider** — SSE framing, `[DONE]` without a `finish_reason`, in-stream error objects, +captive-portal HTML with HTTP 200, HTTP status mapping across nine codes, and free-model detection +including the `0.000000` form the old string comparison got wrong. + +**Storage** — session round-trip with non-ASCII content, corruption quarantine, best-effort writes +when the target can't be written, and the rule that an empty free-model list is never cached. + +**UI** — block splitting across delta boundaries, fences spanning many chunks, unterminated fences, +`Reset()` discarding an abandoned attempt, and cell-width measurement for CJK, emoji, surrogate +pairs and combining marks. + +## Two conventions worth keeping + +**Test the consumer's actual behaviour, not a convenient one.** +`PersistsTheTurnEvenWhenTheConsumerStopsAtTheFinalChunk` deliberately breaks out of the loop on +`IsFinal`, because that is what the console does. An earlier test drained the sequence to +completion and passed against genuinely broken code — persistence had never worked in any build, +and draining hid it completely. + +**Set up the way the app does.** `BuildAsync` calls `ResumeAsync`, because that is what seeds the +system prompt. Skipping it tests an engine in a state the app never reaches. + +## Testing the console + +`AnsiConsole.Create` with an `AnsiConsoleOutput` over a `StringWriter` gives a console you can +assert against: + +```csharp +var output = new StringWriter(); +var console = AnsiConsole.Create(new AnsiConsoleSettings +{ + Ansi = AnsiSupport.No, + ColorSystem = ColorSystemSupport.NoColors, + Out = new AnsiConsoleOutput(output), +}); +``` + +With ANSI off, `TranscriptWriter` streams no raw text and emits no cursor movement, so what remains +is exactly the styled block output — which is what the assertions are about. + +This is also why the writer decides whether it may rewind from **its own console's capabilities** +rather than global state. It was originally global, which made it both untestable and wrong: the +fallback path called `Cursor.Move` and threw without a real console handle. + +## Not covered + +`ConsoleHost`'s REPL loop, the OAuth browser flow, `DpapiKeyStore` (needs a real Windows user +profile), and `CommandRouter` interaction. These need either a live terminal or a network round +trip; they are covered by the manual checks in +[`06-build-and-distribute.md`](06-build-and-distribute.md) instead. + +If you touch the console, run it and look at it — in Windows Terminal *and* in `conhost.exe`. +Several defects here were invisible in review and obvious on screen. diff --git a/docs/architecture/01-layers.md b/docs/architecture/01-layers.md new file mode 100644 index 0000000..e0f9ff8 --- /dev/null +++ b/docs/architecture/01-layers.md @@ -0,0 +1,78 @@ +# Layers + +Three projects. Dependencies point inward, and nothing points back out. + +``` +OpenKey ─────────► OpenKey.Core ◄───────── OpenKey.Providers.OpenRouter +(host) (contracts) (transport) +``` + +## `OpenKey.Core` — the middle + +Engine, rotation policy, storage implementations, and the contracts everything else agrees on. + +**It has zero `PackageReference` entries.** Not "few" — zero. That is the enforcement mechanism for +CLAUDE.md's rule that Core must not depend on a host or a provider: there is no console library to +accidentally call, no HTTP client to reach for, no JSON attribute from a third party to leak into a +persisted shape. Adding a package here should feel like it needs justifying, because it does. + +Targets `net10.0` — not `net10.0-windows`. This matters more than it looks: it is what keeps a +future browser or Android port from inheriting a Windows-only target framework through the shared +layer. + +| Area | Types | +|---|---| +| Contracts | `IChatProvider`, `ChatRequest`, `ChatMessage`, `ChatChunk`, `ModelInfo`, `ChatErrorKind`, `ChatException` | +| Engine | `ChatEngine`, `IRotationPolicy`, `RotationPolicy`, `ITokenCounter` | +| Storage | `IKeyStore`, `ISessionStore`, `IModelCatalog`, `IConfigStore`, `JsonSessionStore`, `JsonModelCatalog`, `JsonConfigStore`, `OpenKeyJsonContext` | +| Paths | `IAppPaths` | + +`IAppPaths` deserves a note. It has one required member, `RootDir`, and derives every filename from +it via default interface members. A test substitutes a temp directory by implementing a single +property, which is why storage tests need no filesystem mocking. + +## `OpenKey.Providers.OpenRouter` — the outside edge + +Everything that knows what OpenRouter's API looks like: URL shapes, headers, SSE framing, the +pricing fields that determine whether a model is free, and the mapping from HTTP status to +`ChatErrorKind`. + +It references Core to implement `IChatProvider` and depends on nothing else. A second provider is a +sibling project, not a modification to this one. + +## `OpenKey` — the host + +The console: `ConsoleHost`, `CommandRouter`, the `Ui/` components, `MarkdownConsoleRenderer`, the +DPAPI key store, and the OAuth flow. + +This is the only project that targets `net10.0-windows`, and the only one that references +Spectre.Console, Markdig, or DPAPI. `DpapiKeyStore` lives here rather than in Core precisely so +that Core can stay platform-neutral — a browser port supplies its own `IKeyStore` backed by Web +Crypto without Core changing at all. + +Composition happens in `Program.cs`, explicitly, with no assembly scanning. One detail is load- +bearing: the provider receives `keys.Load` as a `Func` rather than a key value, so a +`/reset` that replaces the stored key takes effect immediately without rebuilding the container. + +## Tests + +| Project | Scope | Target | +|---|---|---| +| `OpenKey.Core.Tests` | Engine, rotation, storage | `net10.0` | +| `OpenKey.Tests` | Provider, console UI, PKCE | `net10.0-windows` | + +Both reach internals through `InternalsVisibleTo`. The split follows the platform boundary: Core's +tests run anywhere Core does. + +## Why this split earns its keep + +The honest test of a layering scheme is whether it prevents anything. This one does: + +- Rotation logic is testable without HTTP, because `ChatEngine` sees `IChatProvider` and never a + socket. `FakeChatProvider` is 70 lines. +- The console can be rewritten — it was, wholesale — with no change to Core. +- Key storage swaps per platform because nothing above it knows what DPAPI is. + +The part that is *not* yet proven is multi-provider. `IChatProvider.Id` and `DisplayName` exist and +are read by nothing. Until a second provider ships, treat that seam as designed but unexercised — +see [`08-decisions.md`](08-decisions.md). diff --git a/docs/architecture/02-request-lifecycle.md b/docs/architecture/02-request-lifecycle.md new file mode 100644 index 0000000..cdae165 --- /dev/null +++ b/docs/architecture/02-request-lifecycle.md @@ -0,0 +1,111 @@ +# Request lifecycle + +One message, keystroke to rendered reply. + +``` +ReadUserLine ──► CommandRouter ──► ChatEngine.SendAsync ──► OpenRouterProvider + │ handled │ retry loop │ SSE + ▼ ▼ ▼ + (command) TranscriptWriter ◄──────── ChatChunk +``` + +## 1. Reading input + +`ConsoleHost.ReadUserLine` uses `Console.ReadLine`, not a Spectre prompt. Three reasons, all +practical: the Windows console reader supplies arrow keys, Home/End, word jump and F7 history that +Spectre's reader does not implement; it does not throw when output is redirected, which Spectre +prompts now do; and it leaves the remainder of a multi-line paste in the driver buffer. + +That last point matters. Console paste arrives as synthetic keystrokes and a newline reads as +Enter, so the first line submits and the rest lands in the *next* prompt. If one of those lines +begins with `/`, it executes as a command — a pasted transcript containing `/reset` could open the +wipe confirmation with the following line answering it. So after reading a line, OpenKey checks +whether input is already buffered: a human cannot type the next line within milliseconds, so +buffered input means paste, and the remainder is drained and joined into one message. + +## 2. Command or message + +`CommandRouter.HandleAsync` returns `NotACommand` immediately for anything not starting with `/`. +Commands are handled entirely in the host — Core has no notion of them. + +## 3. The retry loop + +`ChatEngine.SendAsync` is an async iterator. The user turn is appended once, before the loop, and +the whole loop is wrapped in `try`/`finally` — legal around `yield`, unlike `try`/`catch` — so that +a failed or cancelled turn removes its own user message instead of leaving it to be persisted on +the next success. + +Up to `MaxAttempts` (5) times: + +1. Fetch free models from `IModelCatalog`. An empty list breaks out with a `ChatException` rather + than reaching `PickAsync`, which used to throw an `InvalidOperationException` that nothing + caught. +2. Move the pinned model to the front, if one is pinned. +3. `IRotationPolicy.PickAsync` chooses the first model not on cooldown. +4. On attempts after the first, emit a chunk with `IsAttemptRestart` set — see below. +5. Trim history to fit the model's context. +6. Open the provider stream and relay chunks. + +### The restart signal + +When an attempt fails part-way, text from it has already reached the consumer. Without a signal, +the console would concatenate the abandoned attempt and the retry, showing the answer twice while +the saved history held it once — screen and history permanently disagreeing. + +So the engine emits `ChatChunk` with `IsAttemptRestart = true` before any text from the new +attempt. `TranscriptWriter.Reset()` erases what was drawn. Providers never set this flag; it is +purely engine-to-consumer. + +### Committing before the last yield + +Success bookkeeping — `MarkSuccess`, appending the assistant turn, saving the session — happens +**before** the final chunk is yielded. + +This ordering is not stylistic. A consumer that stops as soon as it sees `IsFinal` — the natural +way to read this stream, and what `ConsoleHost` does — disposes the iterator at that `yield`. +Anything after it never runs. With the commit placed afterwards, no conversation was ever written +to disk and no model success was ever recorded, in any build up to that point. Draining the +sequence to completion hides the bug entirely, which is why the regression test deliberately breaks +early. + +## 4. Streaming out + +`OpenRouterProvider.StreamChatAsync` reads SSE lines and yields `ChatChunk`s. Each read carries its +own deadline rather than the whole response sharing one — see +[`05-provider-layer.md`](05-provider-layer.md). + +## 5. Rendering + +`ConsoleHost.SendAndRenderAsync` runs in two phases. + +**Phase one** shows a spinner until the first text arrives. The enumerator is created *outside* the +`Status().StartAsync` callback so it survives the handoff; the callback simply returns once it has +a chunk with text. + +The reason for a hard handoff: writing beneath a running spinner works, but only for whole lines. +Spectre's live region repositions the cursor to column 0 before each repaint, so per-token writes +get overwritten. The spinner has to be torn down before streaming starts. + +**Phase two** prints the reply header — model id and elapsed time, known before the first byte, so +the user never stares at nothing — then any rotation note, then hands every chunk to +`TranscriptWriter`. + +## 6. Block-level rendering + +`TranscriptWriter` streams raw text as it arrives and, each time a markdown block completes, erases +those rows and repaints them styled. The still-arriving tail stays plain: styled means settled, raw +means still coming. + +Details, including the three cases where it refuses to erase, are in +[`06-console-host.md`](06-console-host.md). + +## Cancellation + +One `CancellationTokenSource` per turn, held in a field the Ctrl+C handler reads. Mid-turn, Ctrl+C +cancels that reply; at the prompt, it exits. + +The previous design used a single process-lifetime source. Once cancelled it stayed cancelled, so +every subsequent message was born already cancelled and the session was unusable until restart. + +The catch is filtered on `turnCts.IsCancellationRequested` so that an upstream deadline is not +mislabelled as "you cancelled this". diff --git a/docs/architecture/03-rotation-engine.md b/docs/architecture/03-rotation-engine.md new file mode 100644 index 0000000..a2cf1dc --- /dev/null +++ b/docs/architecture/03-rotation-engine.md @@ -0,0 +1,87 @@ +# Rotation engine + +Free models rate-limit constantly. Rotation is what makes that invisible: when one model refuses, +OpenKey moves to another and the user sees a reply rather than an error. + +Normative rules live in [`../04-model-rotation.md`](../04-model-rotation.md). This explains the +design and the parts that are easy to get wrong. + +## State + +`RotationPolicy` keeps a `ModelState` per model id: + +| Field | Meaning | +|---|---| +| `ModelId` | Key | +| `FailureCount` | Consecutive failures; drives backoff | +| `CooldownUntil` | Not usable before this instant | +| `LastErrorKind` | Why it was last cooled down | +| `LastUsedAt` | Last selection | + +Persisted to `rotation.state.json` after each success or failure, so cooldowns survive a restart — +otherwise relaunching would immediately retry a model that just rate-limited you. + +## Selection + +`PickAsync` returns the first candidate whose cooldown has expired. Candidate order comes from the +catalog, with a pinned model moved to the front. + +If everything is cooling down, it finds the soonest and either waits (when the wait is short) or +raises a rate-limit error telling the user roughly how long. An empty candidate list raises a +`ChatException` — deliberately not `InvalidOperationException`, because callers catch the former +and an empty list used to escape as an unhandled crash. + +## Two separate questions + +The subtle part of this design is that "should we retry?" and "is this model to blame?" are *not* +the same question, and treating them as one produces bad behaviour. + +``` +IsTransient(kind) → should we try a different model? +IsModelFault(kind) → should this model be penalised? +``` + +**`NetworkDown` answers no to both.** With no route to the provider every model fails identically. +Retrying burns all five attempts; cooling each one down punishes eight models for an outage none of +them caused — and leaves OpenKey still broken after the network returns. So a network failure stops +immediately, blames nobody, and says "check your connection". + +**`InvalidRequest` answers no to the first.** A request the model cannot accept — context overflow, +unknown model id — cannot succeed by being sent somewhere else unchanged. Retrying it would cool +down every model in turn for the user's fault. + +**`AuthFailure` and `QuotaExhausted`** are fatal for the key, not the model. No rotation helps. + +Only `TransientRateLimit`, `TransientServer` and `MalformedResponse` rotate. + +## Cooldowns + +Base duration by kind, doubled per consecutive failure, capped at five minutes. `AuthFailure` is +effectively permanent — it will not succeed on retry, so there is no point scheduling one. + +A `Retry-After` header, when present, raises the rate-limit cooldown to at least what the server +asked for. Ignoring it means being rate-limited again immediately. + +## Context trimming + +Before each attempt, `BuildMessagesForModel` drops the oldest non-system messages until the +estimated token count fits the chosen model's context, reserving room for the reply. The system +prompt is never dropped. + +Counting goes through `ITokenCounter`. The interface exists because Core has no package references +and a real tokenizer needs a vocabulary, so the host supplies a cl100k-backed implementation and +Core falls back to a four-characters-per-token heuristic when none is given. + +cl100k is a GPT-family encoding while the free tier is mostly Llama, Qwen, DeepSeek and Mistral, so +it is close rather than exact — which is fine, because this only decides how much history to drop. +Trimming happens per attempt rather than once, since rotation can land on a model with a very +different context size. + +## What the user sees + +One grey line: *"Moved past 2 busy models."* + +Not a yellow warning, not one line per attempt, and never a model id or an error-kind name. +Rotation working correctly is not a warning — it is the feature doing its job, and shouting about +it makes a working app look broken. The model that actually answered is already named in the reply +header. diff --git a/docs/architecture/04-storage-and-crypto.md b/docs/architecture/04-storage-and-crypto.md new file mode 100644 index 0000000..4b1ffac --- /dev/null +++ b/docs/architecture/04-storage-and-crypto.md @@ -0,0 +1,83 @@ +# Storage and crypto + +Everything OpenKey keeps lives under `%APPDATA%\OpenKey\`, typically +`C:\Users\\AppData\Roaming\OpenKey`. Normative shapes are in +[`../05-persistence-and-reset.md`](../05-persistence-and-reset.md). + +| File | Owner | Contents | +|---|---|---| +| `key.bin` | `DpapiKeyStore` | DPAPI-encrypted OpenRouter key | +| `session.json` | `JsonSessionStore` | Conversation history | +| `models.cache.json` | `JsonModelCatalog` | Free model list, 24-hour lifetime | +| `rotation.state.json` | `RotationPolicy` | Per-model cooldowns | +| `config.json` | `JsonConfigStore` | Theme and model preferences | + +`config.json` holds `preferredModels` (a single entry is how a pinned model is expressed), `theme`, +and `maxTokens`. It is meant to be hand-editable, so every field is normalised on load rather than +trusted: blank ids dropped, unknown themes reverted, out-of-range token limits reset. A corrupt file +is quarantined like a corrupt session — preferences are never worth failing a launch over. + +## The key + +`ProtectedData.Protect` with `DataProtectionScope.CurrentUser` and a fixed entropy string. The +result is decryptable only by the same Windows account on the same machine — copy `key.bin` +elsewhere and it is inert. + +What DPAPI does *not* protect against is another process running as the same user; it can call +`Unprotect` exactly as OpenKey does. That is the accepted boundary for a single-user desktop app, +and it is stated in [`../../SECURITY.md`](../../SECURITY.md) rather than left implied. + +Save failures here are **not** swallowed, unlike every other store. A key that silently fails to +persist means signing in again on every launch with no explanation, so `Save` raises a +`ChatException` naming the directory and the likely cause. + +## Writes are atomic and best-effort + +Every JSON write goes to `.tmp` and is then `File.Move`d over the target, so a crash mid-write +cannot leave a half-written file where a valid one was. + +Every write except the key is wrapped in a guard for `IOException` and `UnauthorizedAccessException`: + +```csharp +try { /* write */ } +catch (Exception ex) when (ex is IOException or UnauthorizedAccessException) { } +``` + +This is not laziness. `SaveAsync` runs immediately after a reply has been generated but *before* it +is shown. An unguarded throw on a full disk or a read-only roaming profile loses the user a reply +they already waited for. Losing history is the lesser failure, and the reply still reaches the +screen. + +## Corruption heals itself + +`JsonSessionStore.LoadAsync` catches `JsonException` and `IOException`, renames the file to +`session.json.broken-`, and returns null. OpenKey starts with an empty conversation instead of +refusing to start, and the bad file is preserved for diagnosis rather than deleted. + +## Serialization is source-generated + +`OpenKeyJsonContext` covers `SessionSnapshot`, the model cache envelope, and the rotation envelope; +`OpenRouterJsonContext` and `OAuthJsonContext` cover request bodies. Reflection-based +serialization is neither trim- nor AOT-safe, and these shapes are an on-disk contract that changes +rarely and deliberately — exactly what source generation is for. + +Anonymous request bodies had to become named records to make this work, which also removed a +`"temperature": null` that was being sent on every request. + +### One trap worth knowing + +`ChatMessage` carries `[JsonConstructor]`. It has a convenience constructor alongside the full one, +and `System.Text.Json` refuses to guess between two constructors — it throws +`NotSupportedException`, which is *not* a `JsonException` and therefore was not caught by the +corruption handler above. + +The effect: restoring a saved conversation never worked in any build, and the failure was silent. +If you add a second constructor to a persisted type, annotate it. + +## `/reset` + +Clears in-memory state, clears each store, deletes the directory, then re-runs first-run setup. + +If setup is then abandoned there is no key, and returning to the prompt would strand the user: +every message fails auth, the error suggests `/reset`, and `/reset` arrives back at the same place. +So OpenKey exits instead, saying why. diff --git a/docs/architecture/05-provider-layer.md b/docs/architecture/05-provider-layer.md new file mode 100644 index 0000000..25562a4 --- /dev/null +++ b/docs/architecture/05-provider-layer.md @@ -0,0 +1,106 @@ +# Provider layer + +`OpenRouterProvider` is the only code that knows what OpenRouter looks like. Wire format details +are normative in [`../03-openrouter-integration.md`](../03-openrouter-integration.md). + +## The interface + +```csharp +public interface IChatProvider +{ + string Id { get; } + string DisplayName { get; } + Task> ListModelsAsync(CancellationToken ct); + IAsyncEnumerable StreamChatAsync(ChatRequest request, CancellationToken ct); +} +``` + +`Id` and `DisplayName` are read by nothing today. They exist for a provider picker that does not +exist yet — see [`08-decisions.md`](08-decisions.md). + +The key arrives as `Func`, not `string`, so a `/reset` that replaces it takes effect +without rebuilding the DI container. + +## Deadlines are per-read, not per-response + +This is the single most important thing in this file. + +`HttpClient.Timeout` bounds the **entire** operation including reading the response body — even +under `HttpCompletionOption.ResponseHeadersRead`. For a streaming endpoint that is exactly wrong: a +slow model producing a long but perfectly healthy reply gets aborted mid-stream, the abort surfaces +as a cancellation that looks like a network fault, rotation moves on, and the next model dies the +same way at the same deadline. On a slow model the app could never succeed. + +So `HttpClient.Timeout` is `Timeout.InfiniteTimeSpan`, and the provider applies its own: + +| Deadline | Bounds | +|---|---| +| `FirstTokenTimeout` (45s) | Getting response headers, and the wait for the first SSE line | +| `StallTimeout` (30s) | The wait for each subsequent line | +| `ModelListTimeout` (30s) | The whole (non-streaming) `/models` call | + +Each read gets a linked CTS with `CancelAfter`. What is being measured is the wait for the *next* +byte, which is what actually distinguishes a slow model from a dead connection. A model that +streams steadily for ten minutes is fine; one that goes quiet for thirty seconds is not. + +The catch filters on `!ct.IsCancellationRequested` so our own deadline is never reported as the +user cancelling. + +## SSE framing + +Lines are read one at a time. Blank lines and `:` comments are skipped; only `data: ` payloads are +parsed. + +``` +data: {"choices":[{"delta":{"content":"Hel"}}]} +data: {"choices":[{"delta":{"content":"lo"}}]} +data: {"choices":[{"delta":{},"finish_reason":"stop"}]} +data: [DONE] +``` + +### `[DONE]` without a finish reason + +Some models close the stream with `[DONE]` and never send `finish_reason`. Reporting `null` there +made the engine treat a complete reply as unfinished: it discarded the text, cooled the model down, +and retried elsewhere — punishing a model that had answered correctly. + +The provider now remembers the last `finish_reason` it saw and emits `lastFinishReason ?? "stop"`. +`"stop"` is the right default: the server closed the stream cleanly, which is what a normal +completion looks like. + +## Free-model detection + +A model is free if its id ends in `:free`, or if both `pricing.prompt` and `pricing.completion` +parse to zero. + +*Parse*, not string-match. The original compared against `"0"`, `"0.0"` and `"0.00"` exactly, so +the moment OpenRouter formatted a price as `"0.000000"` a free model was silently classified as +paid and vanished from `/models`. Comparing formatted numbers as text is a bug waiting for someone +else's formatter to change. + +## Error mapping + +| HTTP | Kind | Retried? | +|---|---|---| +| 401, 403 | `AuthFailure` | no | +| 402 | `QuotaExhausted` | no | +| 400, 404, 422 | `InvalidRequest` | no | +| 408 | `TransientServer` | yes | +| 429 | `TransientRateLimit` | yes (honours `Retry-After`) | +| 5xx | `TransientServer` | yes | +| other | `MalformedResponse` | yes | + +400/404/422 were previously `MalformedResponse`, which is retryable — so a request no model could +accept was resent five times, cooling down five models on the way. + +## Hostile responses + +`ListModelsAsync` guards both the JSON parse and the `data` property lookup. + +A captive portal — hotel, airport, café Wi-Fi — answers *any* request with its own login page and +HTTP 200. Unguarded, that threw an unhandled `JsonException` during first run, and with no +top-level handler the window closed on the same frame as the stack trace. For software distributed +on a USB stick this is a likely first experience, not an edge case. It now maps to `NetworkDown` +with copy that names the actual cause: you may still need to sign in to this Wi-Fi. + +An in-stream `error` object raises a `ChatException` carrying the server's message. diff --git a/docs/architecture/06-console-host.md b/docs/architecture/06-console-host.md new file mode 100644 index 0000000..02c70c6 --- /dev/null +++ b/docs/architecture/06-console-host.md @@ -0,0 +1,161 @@ +# Console host + +The console is a product surface, not a debug view. A non-technical person double-clicks an `.exe` +off a USB stick; everything below follows from that. + +## Where things live + +| File | Responsibility | +|---|---| +| `Ui/Theme.cs` | Every colour, as Spectre style names | +| `Ui/Glyphs.cs` | Every non-ASCII character, tiered | +| `Ui/ConsoleLayout.cs` | Startup: encoding, width, capabilities | +| `Ui/Components.cs` | Every visible element | +| `Ui/TranscriptWriter.cs` | Streaming reply rendering | +| `Ui/TextWidth.cs` | Terminal cell measurement | +| `MarkdownConsoleRenderer.cs` | Markdown to Spectre | +| `ConsoleHost.cs` | REPL, first run, cancellation | +| `CommandRouter.cs` | Slash commands | + +**A colour or glyph literal outside `Theme` or `Glyphs` is a defect.** Styling used to happen at +call sites and drifted into three different letter cases and four border styles, sometimes inside a +single method. + +## Colour + +> One accent, one neutral, three signals. + +Colour carries meaning, never decoration. Body text is never coloured — it inherits the terminal's +own foreground, because hardcoding white breaks every light-background console. + +| Role | Style | For | +|---|---|---| +| Brand | `aqua` | Wordmark, AI label, caret, commands, model ids | +| Strong | `bold` | Names: username, headers, markdown strong | +| Muted | `grey` | Hints, elapsed, borders, secondary detail | +| Ok / Warn / Danger | `green` / `yellow` / `red` | The glyph and card chrome only | + +Only the 16 base ANSI colours are used. Legacy conhost downsamples anything richer, and the nicer +greys (`grey19`, `grey23`) land on black or silver depending on the user's scheme — so they either +vanish or invert. + +That restriction is also why there is no capability-degradation machinery here: with a base-16 +palette there is nothing to downsample, and Spectre strips colour itself under `NO_COLOR`. A more +elaborate theme object would be solving a problem this palette does not have. + +`/theme` switches between `default`, `dark`, `light` and `mono`, persisted to `config.json` and +applied once at startup. + +**Colour is never the only signal.** Roughly 8% of men cannot reliably separate red from green, so +every state that uses them also carries a glyph or a worded title — `✓` versus `✗`, and error cards +that name the problem in their header. `mono` is the standing test of that claim: a unit test +asserts no hue survives it, so anything that starts relying on colour alone fails the build. + +No background colours. Inline code used to be `white on grey23`, which downsampled to white-on-black +— invisible on light schemes, identical to body text on dark ones. The one style meant to make code +stand out did nothing. + +## Glyphs + +Windows Terminal has font fallback and renders anything. The GDI-rendered legacy conhost does not: +a glyph missing from Consolas draws a box. So the tier is chosen conservatively — full Unicode only +when the codepage is UTF-8 *and* `WT_SESSION` is set. + +| Role | Unicode | ASCII | Note | +|---|---|---|---| +| Caret | `❯` | `>` | In neither CP437 nor Consolas | +| Ok / Fail | `✓` `✗` | `+` `x` | Not in CP437 | +| Spinner | Braille dots | `SimpleDots` | **Ran on every single turn** | +| Borders | Square | Square | Rounded is outside CP437 | +| Bullet | `-` | `-` | A hyphen is calmer *and* safer than `•` | + +The spinner was the most likely visible breakage in the whole app: Braille U+28xx, absent from +Consolas, displayed on every message. + +## Streaming + +`TranscriptWriter` writes raw text as it arrives; when a markdown block completes it erases those +rows and repaints them styled. The unstyled tail is intentional — styled means settled, raw means +still arriving. + +### Why not `LiveDisplay` + +It is the obvious tool and it is wrong here. Reading Spectre 0.57.2's `LiveRenderable`: + +- The region is clamped to the viewport, and overflow lines are **discarded**, not scrolled. +- On shrink it issues `EraseInDisplay(2)` followed by `ClearScrollback()` — which would erase the + conversation. +- Cursor control is dropped entirely when output is redirected, and unlike `Status`/`Progress` it + has no fallback renderer, so every frame appends. + +For a scrolling transcript, destroying scrollback ends the discussion. + +### The rewind + +`\r`, `CursorUp(n)`, `EraseInDisplay(0)` — emitted through `ControlCode` so it stays inside +Spectre's capability gate. Never `EraseInDisplay(2)`, never `ClearScrollback`, and always relative +`CursorUp` rather than an absolute position, which lands wrong if the region straddled a scroll. + +It **refuses to rewind** in three cases, leaving the raw text in place instead: + +1. **Taller than the viewport** — those rows are already in scrollback, where no escape sequence + reaches. Erasing what remains would tear the output in half. +2. **The terminal was resized** — the row count is stale, and a wrong rewind eats unrelated + transcript above. +3. **No ANSI, or output redirected** — nothing can be erased, so nothing is streamed raw either; + the styled block render is the only output. + +Losing a restyle is cheap. Eating the conversation is not. + +Code fences are buffered rather than streamed raw, since they are the most likely block to outgrow +the viewport. + +### Counting rows + +`TextWidth` measures terminal cells, not characters. CJK and emoji occupy two, combining marks and +joiners occupy none, and a surrogate pair is two chars but one glyph. Getting this wrong makes the +rewind erase the wrong number of rows — which damages the transcript above, so it is worth the +care. + +Wrapping is computed at `width - 1` because terminals disagree about a glyph landing exactly on the +last column: conhost wraps immediately, Windows Terminal defers. Staying a column short agrees with +both. + +Spectre has an equivalent calculator, but it is `internal` in 0.57.2. + +## Spinner handoff + +Writing beneath a running spinner works — but only for whole lines, because the live region +repositions the cursor to column 0 before each repaint. Per-token writes get overwritten. + +So phase one runs the spinner until the first text arrives, with the enumerator created *outside* +the callback so it survives; phase two streams freely after teardown. + +## Capabilities + +`ConsoleLayout.Rich` is `caps.Ansi && caps.Interactive`. Both halves matter: Spectre 0.55 disables +ANSI when stdout is redirected, and makes `Interactive` false if *any* standard stream is +redirected — at which point its prompts throw. That throw used to land in a bare `catch` and exit +the REPL silently. + +Width is clamped to `[60, 100]`. Without the cap, a maximized 200-column terminal stretches a +three-line snippet across the whole screen and no two machines render a reply the same way. + +## Voice + +Sentence case. No `DPAPI`, `OAuth`, `PKCE`, `429`, or `ChatErrorKind` anywhere a user can see — +those were the most developer-tool-looking thing in the app and told the user nothing actionable. + +Every error is a card with two parts: what happened, and what to do next. **A card with no next +step is a bug**, because it leaves someone at a dead end whose only escape is closing the window. + +`/reset` states that it erases the conversation as well as the key *before* asking to confirm, and +never defaults to yes. + +## Exit hold + +If `GetConsoleProcessList` reports OpenKey is the console's only client, it was double-clicked and +the window dies with the process. In that case OpenKey waits for a keypress before exiting. + +Without this, the goodbye line and every fatal error were unreadable by construction — nobody in +the target audience had ever seen either. diff --git a/docs/architecture/07-error-taxonomy.md b/docs/architecture/07-error-taxonomy.md new file mode 100644 index 0000000..2e5b3ad --- /dev/null +++ b/docs/architecture/07-error-taxonomy.md @@ -0,0 +1,66 @@ +# Error taxonomy + +`ChatErrorKind` is a contract surface — see +[`../01-architecture.md`](../01-architecture.md#error-taxonomy). Adding a member propagates to every +implementation, including future ports. + +Everything a provider can fail with maps onto one of these, and rotation is driven entirely by +them. + +## The full table + +| Kind | Cause | Retry elsewhere? | Model's fault? | Cooldown | +|---|---|---|---|---| +| `TransientRateLimit` | HTTP 429 | yes | yes | 60s base, honours `Retry-After` | +| `TransientServer` | 5xx, 408, stalled stream | yes | yes | 30s base | +| `MalformedResponse` | Unparseable payload | yes | yes | 10s base | +| `NetworkDown` | No route, DNS, captive portal | **no** | **no** | none | +| `AuthFailure` | 401, 403 | no | n/a | permanent | +| `QuotaExhausted` | 402 | no | n/a | n/a | +| `InvalidRequest` | 400, 404, 422 | **no** | n/a | n/a | + +Two columns, not one, because "should we try another model?" and "is this model to blame?" are +different questions. Conflating them is what produced the two worst behaviours this taxonomy now +prevents. + +## Why `NetworkDown` answers no to both + +When there is no route to OpenRouter, every model fails identically. Rotating burns all five +attempts and puts eight models on cooldown for an outage none of them caused — so OpenKey stays +broken *after* the network comes back, which is the part users actually notice. + +It fails fast instead, blames nobody, and says to check the connection. + +## Why `InvalidRequest` exists + +Before it, a request no model could accept — context overflow, a model id that no longer exists — +mapped to `MalformedResponse`, which is retryable. So OpenKey resent an identical, impossible +request to five models in turn and cooled down every one of them. + +The request is at fault, not the model. Retrying unchanged cannot succeed. + +## What the user sees + +Copy lives in `ConsoleHost.ShowChatError`. Enum names never reach the screen; every card names a +next step. + +| Kind | Card | Next step | +|---|---|---| +| `AuthFailure` | Danger — "Your key was refused" | `/reset` to sign in again, *and* that it erases history | +| `QuotaExhausted` | Danger — "This key is out of credit" | Add credit, or wait for the allowance to renew | +| `InvalidRequest` | Danger — "The model refused this message" | Shorter message, or `/models` | +| `NetworkDown` | Warn — "Can't reach OpenRouter" | Check the connection and resend | +| `TransientRateLimit` | Warn — "Every free model is busy right now" | Wait and resend, or `/models` | +| other | Warn — "That didn't go through" | Resend, or `/models` | + +Danger means it will not fix itself. Warn means it might. That distinction is the entire reason +there are two card styles — a wall of red reads as panic and stops carrying information. + +## Adding a kind + +1. Update `ChatErrorKind` and [`../01-architecture.md`](../01-architecture.md). +2. Decide `IsTransient` and `IsModelFault` **separately**. +3. Map it in `OpenRouterProvider.MapHttpError`. +4. Add copy in `ShowChatError`, with a next step. +5. Add a cooldown in `RotationPolicy.MarkFailure` if it is a model fault. +6. Update every other implementation — this is a contract change and needs approval first. diff --git a/docs/architecture/08-decisions.md b/docs/architecture/08-decisions.md new file mode 100644 index 0000000..152a973 --- /dev/null +++ b/docs/architecture/08-decisions.md @@ -0,0 +1,152 @@ +# Decisions + +What was chosen, what was rejected, and the evidence. Written so that reasonable-looking ideas that +turn out to be wrong don't get re-proposed every six months. + +--- + +## Keep `IChatProvider`; do not adopt `IChatClient` as the contract + +**Status:** decided · **Revisit:** Phase 3 + +`Microsoft.Extensions.AI` (stable) offers `IChatClient`, which covers roughly what `IChatProvider` +covers and adds tool-calling middleware — useful for Phase 3 — plus a broad provider ecosystem, +useful for Phase 5. + +It is nonetheless rejected **as the contract**, because `IChatProvider` is normative across a +planned browser PWA and Android app. `IChatClient` is .NET-only; a TypeScript port cannot implement +it. Adopting it as the boundary breaks the cross-UI guarantee that is the reason the contract docs +exist at all. + +The move that keeps both benefits is to implement `IChatProvider` **over** an `IChatClient` inside +the provider project only. The portable contract survives, .NET gets the middleware, and no other +layer notices. That is the backlog entry. + +--- + +## `LiveDisplay` is not usable for the transcript + +**Status:** decided, with evidence · **Revisit:** if Spectre changes the overflow model + +The obvious tool for streaming output, and wrong. From Spectre 0.57.2's `LiveRenderable`: + +- Height is clamped to the viewport and overflow lines are **discarded**, not scrolled. +- On shrink it emits `EraseInDisplay(2)` then `ClearScrollback()` — it would erase the conversation. +- With output redirected, cursor control is dropped and every frame appends. Unlike + `Status`/`Progress` it has no fallback renderer. + +`TranscriptWriter` drives the cursor directly instead, and declines to erase in the three cases +where erasing would be wrong. See [`06-console-host.md`](06-console-host.md). + +--- + +## No `IHttpClientFactory`, no Polly + +**Status:** decided · **Revisit:** never, unless rotation is redesigned + +Resilience here is **model-level**, not transport-level. `RotationPolicy` counts failures per model +and schedules cooldowns from them. + +A transport retry policy fires underneath that: the provider silently retries, the engine sees one +outcome instead of three, failure counts and cooldowns no longer describe reality, and rotation +starts making decisions on corrupted data. The two mechanisms cannot both own retry. + +`HttpClient` is a singleton with an infinite timeout — see +[`05-provider-layer.md`](05-provider-layer.md) for why the timeout must not be finite. + +--- + +## No `Microsoft.Extensions.Hosting` + +**Status:** decided + +`docs/02-phase1-build.md` originally prescribed it. A REPL needs no generic host, no hosted-service +lifetime, and no configuration binding. `Program.cs` composes about a dozen services explicitly. + +The code was right and the doc was wrong; the doc has been corrected. + +--- + +## Hand-rolled SSE reader + +**Status:** decided · **Revisit:** when `System.Net.ServerSentEvents` ships stable + +`System.Net.ServerSentEvents` exists but is preview-only (`11.0.0-preview.6` at time of writing). +A preview dependency in a click-and-play binary is not worth ~40 lines of line-reading, especially +when those lines also carry the per-read deadline logic. + +--- + +## `System.Text.Json` source generation + +**Status:** decided + +Reflection-based serialization is neither trim- nor AOT-safe, and it silently pulls in machinery +that never appears in a build log until AOT is attempted. + +Enabling `IsAotCompatible` surfaced the reflection sites immediately as build errors. Anonymous +request bodies became named records; persisted shapes moved to generated contexts. + +One trap is documented in [`04-storage-and-crypto.md`](04-storage-and-crypto.md): a type with two +constructors makes STJ throw `NotSupportedException`, which is not a `JsonException` and therefore +escapes corruption handling. It meant session restore had never worked. + +--- + +## NativeAOT: not now, but the old reason is dead + +**Status:** watching + +`docs/06-build-and-distribute.md` rejected AOT because Spectre.Console used reflection. Measured +from the shipped assemblies: `IsTrimmable` is **absent** in Spectre 0.49.1 and **present** in +0.55.2. That reason expired. + +The other stated blocker, `System.Text.Json` reflection, is gone as of the source-generation work. +`IsAotCompatible` is on across all three projects and the tree builds warning-clean, which also +answers the open question about Markdig. + +The prize is real for a USB-distributed app: roughly 42 MB → 15–20 MB, no extract-to-temp on first +run, faster startup. What remains is measurement, not a known obstacle. + +--- + +## Base-16 palette instead of a degradable theme object + +**Status:** decided + +A richer design — a theme record degraded once against live capabilities — was considered. It +solves problems this palette does not have: with only the 16 base ANSI colours there is nothing to +downsample, and Spectre strips colour itself under `NO_COLOR`. + +Glyphs and borders are the real degradation axis, and they are handled by a tier check in +`Ui/Glyphs.cs`. The simpler design is the correct one here; the elaborate one would be machinery +without a job. + +--- + +## `Console.ReadLine` instead of a Spectre prompt for chat input + +**Status:** decided + +Spectre's reader handles Enter, Tab, Backspace and printable characters, and silently drops +everything else — including arrow keys. `Console.ReadLine` gets the Windows console's own line +editor free: arrows, Home/End, word jump, F7 history. + +It also cannot throw when output is redirected, which Spectre prompts now do, and it leaves the +remainder of a multi-line paste in the driver buffer where it can be drained rather than executed +as commands. + +`TextPrompt` is kept for key entry, where `.Secret()` masking is the whole point. + +--- + +## The multi-provider seam is designed but unproven + +**Status:** acknowledged + +`IChatProvider.Id` and `DisplayName` are read by nothing. One provider exists. An abstraction with +a single implementation has not been tested against reality, whatever its shape suggests. + +The backlog therefore sequences the **Anthropic** provider before the Claude Code subprocess +provider: it exercises the same seam more honestly, since a subprocess provider could be made to +work around a bad abstraction in ways an HTTP one cannot. diff --git a/docs/architecture/README.md b/docs/architecture/README.md new file mode 100644 index 0000000..622e86f --- /dev/null +++ b/docs/architecture/README.md @@ -0,0 +1,50 @@ +# Architecture reference + +Explanatory companion to [`../01-architecture.md`](../01-architecture.md), which stays the +normative contract. Nothing here overrides it: where the two disagree, `01` wins and this folder is +wrong. + +These documents describe the C# desktop implementation as it actually is. Where a future browser or +Android port must match, that requirement lives in the contract docs, not here. + +| Document | Covers | +|---|---| +| [01-layers.md](01-layers.md) | The three projects, why the split exists, what enforces it | +| [02-request-lifecycle.md](02-request-lifecycle.md) | One message end to end, keystroke to rendered reply | +| [03-rotation-engine.md](03-rotation-engine.md) | Model selection, cooldowns, what counts as a model's fault | +| [04-storage-and-crypto.md](04-storage-and-crypto.md) | `%APPDATA%` layout, DPAPI, atomic writes, corruption recovery | +| [05-provider-layer.md](05-provider-layer.md) | OpenRouter wire format, SSE framing, deadlines, error mapping | +| [06-console-host.md](06-console-host.md) | Streaming render, theming, capability degradation, cancellation | +| [07-error-taxonomy.md](07-error-taxonomy.md) | Every `ChatErrorKind`: cause, retry policy, what the user sees | +| [08-decisions.md](08-decisions.md) | Decisions and explicit rejections, with the evidence | + +## The shape of it + +``` + ConsoleHost ──────────► CommandRouter (src/OpenKey) + │ │ + │ TranscriptWriter │ + ▼ ▼ + ┌──────────────────────────────────┐ + │ ChatEngine │ (src/OpenKey.Core) + │ retry · rotate · trim · persist │ + └──────────────────────────────────┘ + │ │ │ + ▼ ▼ ▼ + IChatProvider IRotation ISessionStore + │ Policy IModelCatalog + │ IKeyStore + ▼ + OpenRouterProvider (src/OpenKey.Providers.OpenRouter) + │ + ▼ + api.openrouter.ai +``` + +Dependencies point inward. `OpenKey.Core` names no host and no provider, and has no package +references at all — that absence is what actually enforces the rule, not a convention. + +## If you read one thing + +[`08-decisions.md`](08-decisions.md). It records what was tried, what was rejected, and why — +including several approaches that look obviously correct and are not. diff --git a/global.json b/global.json new file mode 100644 index 0000000..8287d38 --- /dev/null +++ b/global.json @@ -0,0 +1,6 @@ +{ + "sdk": { + "version": "10.0.201", + "rollForward": "latestFeature" + } +} diff --git a/src/OpenKey.Core/Engine/ChatEngine.cs b/src/OpenKey.Core/Engine/ChatEngine.cs index d5d82b2..08b64b2 100644 --- a/src/OpenKey.Core/Engine/ChatEngine.cs +++ b/src/OpenKey.Core/Engine/ChatEngine.cs @@ -8,12 +8,24 @@ public sealed class ChatEngine { private const int MaxAttempts = 5; private const int ResponseTokenReserve = 1024; + + /// + /// Ceiling on a single message, across every attempt. + /// + /// The provider bounds each individual read, but five attempts could still stack into several + /// minutes of a user staring at a spinner. This bounds the sum: once it is spent, the turn + /// stops rotating and reports rather than starting another attempt. + /// + /// + private static readonly TimeSpan TurnBudget = TimeSpan.FromMinutes(2); private const string DefaultSystemPrompt = "You are a helpful assistant."; private readonly IChatProvider _provider; private readonly IRotationPolicy _rotation; private readonly IModelCatalog _catalog; private readonly ISessionStore _sessions; + private readonly IConfigStore _config; + private readonly ITokenCounter _tokens; private readonly List _turns = new(); @@ -21,17 +33,38 @@ public ChatEngine( IChatProvider provider, IRotationPolicy rotation, IModelCatalog catalog, - ISessionStore sessions) + ISessionStore sessions, + IConfigStore config, + ITokenCounter? tokens = null) { _provider = provider; _rotation = rotation; _catalog = catalog; _sessions = sessions; + _config = config; + _tokens = tokens ?? new HeuristicTokenCounter(); + PreferredModelId = config.Current.PinnedModel; } public ModelInfo? ActiveModel { get; private set; } - public string? PreferredModelId { get; set; } + /// + /// Model to try first, or null to let rotation choose. Persisted, so a pin survives a restart — + /// it was previously session-scoped only because there was nowhere to store it. + /// + public string? PreferredModelId + { + get; + set + { + if (field == value) return; + field = value; + _config.Save(_config.Current.WithPinnedModel(value)); + } + } + + /// The last message the user sent, for /retry. Null before the first turn. + public string? LastUserMessage { get; private set; } public IReadOnlyList Turns => _turns; @@ -63,135 +96,203 @@ public async IAsyncEnumerable SendAsync( string userText, [EnumeratorCancellation] CancellationToken ct) { - _turns.Add(new ChatMessage(ChatMessage.UserRole, userText)); + var userTurn = new ChatMessage(ChatMessage.UserRole, userText); + _turns.Add(userTurn); + LastUserMessage = userText; + var userTurnIndex = _turns.Count - 1; + var succeeded = false; + + // try/finally — legal around `yield`, unlike try/catch — so that a failed or cancelled + // turn does not leave its user message stranded in history to be persisted later. + try + { + ChatException? lastError = null; + var startedAt = DateTimeOffset.UtcNow; - ChatException? lastError = null; + for (int attempt = 1; attempt <= MaxAttempts; attempt++) + { + ct.ThrowIfCancellationRequested(); - for (int attempt = 1; attempt <= MaxAttempts; attempt++) - { - ct.ThrowIfCancellationRequested(); + // Checked between attempts, never mid-stream: a reply that is actively arriving is + // working, however long it has taken, and cutting it off would waste it. + if (attempt > 1 && DateTimeOffset.UtcNow - startedAt > TurnBudget) + { + lastError ??= new ChatException( + ChatErrorKind.TransientServer, + "Gave up after trying several models."); + break; + } - var candidates = await _catalog.GetFreeModelsAsync(ct); + var candidates = await _catalog.GetFreeModelsAsync(ct); - if (PreferredModelId is { } pref) - { - var list = candidates.ToList(); - var idx = list.FindIndex(m => m.Id == pref); - if (idx > 0) + if (candidates.Count == 0) { - var picked = list[idx]; - list.RemoveAt(idx); - list.Insert(0, picked); - candidates = list; + // Guard before PickAsync, which throws a bare InvalidOperationException on an + // empty list — a type nothing upstream catches, so it reached the user as a crash. + lastError = new ChatException( + ChatErrorKind.TransientServer, + "No free models are available right now."); + break; } - } - ModelInfo model; - try - { - model = await _rotation.PickAsync(candidates, ct); - } - catch (ChatException ex) - { - lastError = ex; - break; - } + if (PreferredModelId is { } pref) + { + var list = candidates.ToList(); + var idx = list.FindIndex(m => m.Id == pref); + if (idx > 0) + { + var picked = list[idx]; + list.RemoveAt(idx); + list.Insert(0, picked); + candidates = list; + } + } - ActiveModel = model; + ModelInfo model; + try + { + model = await _rotation.PickAsync(candidates, ct); + } + catch (ChatException ex) + { + lastError = ex; + break; + } - var assistantBuilder = new System.Text.StringBuilder(); - string? finishReason = null; - ChatException? thisAttemptError = null; + // Tell the consumer to discard whatever the previous attempt yielded, before any + // text from this one arrives. Without it a mid-reply rotation renders the answer + // twice concatenated while the persisted session stores it once. + if (attempt > 1) + { + yield return new ChatChunk( + string.Empty, IsFinal: false, FinishReason: null, IsAttemptRestart: true); + } - var messages = BuildMessagesForModel(model); - var request = new ChatRequest(model.Id, messages); + ActiveModel = model; - IAsyncEnumerator? enumerator = null; - try - { - enumerator = _provider.StreamChatAsync(request, ct).GetAsyncEnumerator(ct); - } - catch (ChatException ex) - { - thisAttemptError = ex; - } + var assistantBuilder = new System.Text.StringBuilder(); + string? finishReason = null; + ChatException? thisAttemptError = null; - if (enumerator is not null) - { + var messages = BuildMessagesForModel(model); + var request = new ChatRequest(model.Id, messages, _config.Current.MaxTokens); + + IAsyncEnumerator? enumerator = null; try { - while (true) + enumerator = _provider.StreamChatAsync(request, ct).GetAsyncEnumerator(ct); + } + catch (ChatException ex) + { + thisAttemptError = ex; + } + + if (enumerator is not null) + { + try { - ChatChunk? chunk = null; - try - { - if (!await enumerator.MoveNextAsync()) break; - chunk = enumerator.Current; - } - catch (ChatException ex) + while (true) { - thisAttemptError = ex; - break; - } - - if (chunk is null) break; + ChatChunk? chunk = null; + try + { + if (!await enumerator.MoveNextAsync()) break; + chunk = enumerator.Current; + } + catch (ChatException ex) + { + thisAttemptError = ex; + break; + } + + if (chunk is null) break; + + if (!string.IsNullOrEmpty(chunk.DeltaText)) + assistantBuilder.Append(chunk.DeltaText); + + if (chunk.IsFinal) + { + finishReason = chunk.FinishReason; + + // Commit BEFORE yielding the final chunk. A consumer that stops + // enumerating as soon as it sees IsFinal — which is the natural + // way to consume this, and what the console host does — disposes + // the iterator at the yield, so anything after it never runs. + // Persisting afterwards meant the reply was shown but never saved + // and the model's success never recorded. + if (thisAttemptError is null && finishReason is not null) + { + await CommitTurnAsync(model, assistantBuilder.ToString(), ct); + succeeded = true; + } + + yield return chunk; + break; + } - if (!string.IsNullOrEmpty(chunk.DeltaText)) - assistantBuilder.Append(chunk.DeltaText); - - if (chunk.IsFinal) - { - finishReason = chunk.FinishReason; yield return chunk; - break; } - - yield return chunk; + } + finally + { + await enumerator.DisposeAsync(); } } - finally - { - await enumerator.DisposeAsync(); - } - } - if (thisAttemptError is null && finishReason is not null) - { - _rotation.MarkSuccess(model.Id); - var assistantText = assistantBuilder.ToString(); - _turns.Add(new ChatMessage(ChatMessage.AssistantRole, assistantText)); - await _sessions.SaveAsync( - new SessionSnapshot(model.Id, DateTimeOffset.UtcNow, _turns.ToArray()), - ct); - yield break; - } + if (succeeded) yield break; - if (thisAttemptError is not null) - { - _rotation.MarkFailure(model.Id, thisAttemptError.Kind, thisAttemptError.RetryAfterHint); - OnRotation?.Invoke($"{model.Id} → {thisAttemptError.Kind}"); - - if (!IsTransient(thisAttemptError.Kind)) + if (thisAttemptError is not null) { + // Don't penalise a model for the user's network being down. + if (IsModelFault(thisAttemptError.Kind)) + { + _rotation.MarkFailure( + model.Id, thisAttemptError.Kind, thisAttemptError.RetryAfterHint); + OnRotation?.Invoke($"{model.Id} → {thisAttemptError.Kind}"); + } + lastError = thisAttemptError; - break; + if (!IsTransient(thisAttemptError.Kind)) break; + continue; } - lastError = thisAttemptError; - continue; + // No error but stream ended without IsFinal — treat as malformed + var malformed = new ChatException( + ChatErrorKind.MalformedResponse, + "Stream ended without final chunk."); + _rotation.MarkFailure(model.Id, malformed.Kind, null); + OnRotation?.Invoke($"{model.Id} → {malformed.Kind}"); + lastError = malformed; } - // No error but stream ended without IsFinal — treat as malformed - var malformed = new ChatException( - ChatErrorKind.MalformedResponse, - "Stream ended without final chunk."); - _rotation.MarkFailure(model.Id, malformed.Kind, null); - lastError = malformed; + throw lastError ?? new ChatException( + ChatErrorKind.TransientServer, + "All free models failed after retries."); + } + finally + { + // Reference check rather than value equality: two identical messages differ only by + // timestamp, and removing the wrong one would silently corrupt history. + if (!succeeded + && userTurnIndex < _turns.Count + && ReferenceEquals(_turns[userTurnIndex], userTurn)) + { + _turns.RemoveAt(userTurnIndex); + } } + } - throw lastError ?? new ChatException( - ChatErrorKind.TransientServer, - "All free models failed after retries."); + /// + /// Records a completed turn: the model succeeded, the assistant reply joins the conversation, + /// and the session is written to disk. + /// + private async Task CommitTurnAsync(ModelInfo model, string assistantText, CancellationToken ct) + { + _rotation.MarkSuccess(model.Id); + _turns.Add(new ChatMessage(ChatMessage.AssistantRole, assistantText)); + await _sessions.SaveAsync( + new SessionSnapshot(model.Id, DateTimeOffset.UtcNow, _turns.ToArray()), + ct); } private void ResetTurnsToSystemOnly() @@ -205,7 +306,7 @@ private List BuildMessagesForModel(ModelInfo model) var max = Math.Max(2048, model.ContextLength - ResponseTokenReserve); var trimmed = new List(_turns); - while (EstimateTokens(trimmed) > max && trimmed.Count > 2) + while (_tokens.Count(trimmed) > max && trimmed.Count > 2) { int dropIdx = trimmed[0].Role == ChatMessage.SystemRole ? 1 : 0; trimmed.RemoveAt(dropIdx); @@ -222,9 +323,27 @@ internal static int EstimateTokens(IEnumerable msgs) return total; } + /// + /// Whether a failure is worth retrying on a different model. + /// + /// is deliberately excluded: with no route to the + /// provider, every model fails identically, so rotating burns all five attempts and leaves + /// every model on cooldown for a fault that has nothing to do with any of them — the app + /// would then still be broken after the network came back. + /// + /// + /// is excluded because the request, not the model, + /// is at fault; an identical retry elsewhere cannot succeed. + /// + /// internal static bool IsTransient(ChatErrorKind k) => k is ChatErrorKind.TransientRateLimit or ChatErrorKind.TransientServer - or ChatErrorKind.NetworkDown or ChatErrorKind.MalformedResponse; + + /// + /// Whether a failure should count against the model itself. A model is not at fault for the + /// user's network being down, so cooling it down would penalise it for an unrelated outage. + /// + internal static bool IsModelFault(ChatErrorKind k) => k is not ChatErrorKind.NetworkDown; } diff --git a/src/OpenKey.Core/Engine/ITokenCounter.cs b/src/OpenKey.Core/Engine/ITokenCounter.cs new file mode 100644 index 0000000..3bfdd8e --- /dev/null +++ b/src/OpenKey.Core/Engine/ITokenCounter.cs @@ -0,0 +1,40 @@ +using OpenKey.Core.Providers; + +namespace OpenKey.Core.Engine; + +/// +/// Counts tokens for context trimming. +/// +/// An interface rather than a direct dependency because OpenKey.Core deliberately has no +/// package references — that absence is what stops a host or provider concern leaking into the +/// shared layer. A real tokenizer needs a vocabulary package, so it is supplied from outside. +/// +/// +public interface ITokenCounter +{ + int Count(string text); + + /// + /// Tokens for a whole conversation, including the small per-message overhead every chat API + /// adds for role framing. + /// + int Count(IEnumerable messages) + { + var total = 0; + foreach (var m in messages) total += Count(m.Role) + Count(m.Content) + 4; + return total; + } +} + +/// +/// Fallback used when no real tokenizer is supplied: roughly four characters per token. +/// +/// Crude, and deliberately so — it is only ever used to decide how much history to drop, where +/// over-estimating costs a little context and under-estimating costs a rejected request. It errs +/// high for that reason. +/// +/// +public sealed class HeuristicTokenCounter : ITokenCounter +{ + public int Count(string text) => string.IsNullOrEmpty(text) ? 0 : (text.Length / 4) + 1; +} diff --git a/src/OpenKey.Core/Engine/RotationPolicy.cs b/src/OpenKey.Core/Engine/RotationPolicy.cs index f648e5d..c44406e 100644 --- a/src/OpenKey.Core/Engine/RotationPolicy.cs +++ b/src/OpenKey.Core/Engine/RotationPolicy.cs @@ -2,17 +2,12 @@ using System.Text.Json; using OpenKey.Core.AppPaths; using OpenKey.Core.Providers; +using OpenKey.Core.Storage; namespace OpenKey.Core.Engine; public sealed class RotationPolicy : IRotationPolicy { - private static readonly JsonSerializerOptions JsonOpts = new() - { - WriteIndented = true, - PropertyNamingPolicy = JsonNamingPolicy.CamelCase, - }; - private static readonly TimeSpan MaxCooldown = TimeSpan.FromMinutes(5); private readonly IAppPaths _paths; @@ -26,8 +21,14 @@ public RotationPolicy(IAppPaths paths) public async Task PickAsync(IReadOnlyList candidates, CancellationToken ct) { + // ChatException, not InvalidOperationException: callers catch the former, so the latter + // escaped as an unhandled crash whenever the free-model list came back empty. if (candidates.Count == 0) - throw new InvalidOperationException("No candidate models available."); + { + throw new ChatException( + ChatErrorKind.TransientServer, + "No free models are available right now."); + } var now = DateTimeOffset.UtcNow; @@ -144,7 +145,7 @@ private ModelState GetOrCreate(string id) try { using var stream = File.OpenRead(path); - var env = JsonSerializer.Deserialize(stream, JsonOpts); + var env = JsonSerializer.Deserialize(stream, OpenKeyJsonContext.Default.StateEnvelope); return env?.States is null ? null : new Dictionary(env.States, StringComparer.Ordinal); @@ -157,15 +158,24 @@ private ModelState GetOrCreate(string id) private void SaveToDisk() { - _paths.EnsureRoot(); - var path = _paths.RotationStateFile; - var tmp = path + ".tmp"; - var env = new StateEnvelope(_states); - using (var stream = File.Create(tmp)) + // Best-effort. This runs from MarkSuccess/MarkFailure in the middle of a turn, so an + // unguarded IOException here would surface as a crash on an otherwise healthy reply. + // Cooldown state is a convenience; losing it costs one wasted retry after a restart. + try + { + _paths.EnsureRoot(); + var path = _paths.RotationStateFile; + var tmp = path + ".tmp"; + var env = new StateEnvelope(_states); + using (var stream = File.Create(tmp)) + { + JsonSerializer.Serialize(stream, env, OpenKeyJsonContext.Default.StateEnvelope); + } + File.Move(tmp, path, overwrite: true); + } + catch (Exception ex) when (ex is IOException or UnauthorizedAccessException) { - JsonSerializer.Serialize(stream, env, JsonOpts); } - File.Move(tmp, path, overwrite: true); } public sealed class ModelState @@ -177,5 +187,6 @@ public sealed class ModelState public DateTimeOffset LastUsedAt { get; set; } } - private sealed record StateEnvelope(IReadOnlyDictionary States); + // internal, not private: OpenKeyJsonContext must be able to name it. + internal sealed record StateEnvelope(IReadOnlyDictionary States); } diff --git a/src/OpenKey.Core/OpenKey.Core.csproj b/src/OpenKey.Core/OpenKey.Core.csproj index d2f4542..4848f4f 100644 --- a/src/OpenKey.Core/OpenKey.Core.csproj +++ b/src/OpenKey.Core/OpenKey.Core.csproj @@ -3,6 +3,7 @@ OpenKey.Core OpenKey.Core false + true diff --git a/src/OpenKey.Core/Providers/ChatErrorKind.cs b/src/OpenKey.Core/Providers/ChatErrorKind.cs index 3c710a3..77be1e8 100644 --- a/src/OpenKey.Core/Providers/ChatErrorKind.cs +++ b/src/OpenKey.Core/Providers/ChatErrorKind.cs @@ -1,11 +1,38 @@ namespace OpenKey.Core.Providers; +/// +/// Normative error taxonomy — see docs/01-architecture.md § "Error taxonomy". +/// Every provider maps its transport failures onto these, and rotation policy is driven +/// entirely by them. Adding a member is a contract change that propagates to every impl. +/// public enum ChatErrorKind { + /// Rate limited. Retryable on a different model. TransientRateLimit, + + /// Upstream 5xx or timeout. Retryable on a different model. TransientServer, + + /// Key is missing, invalid, or revoked. Fatal — no model will accept it. AuthFailure, + + /// Credit or quota exhausted. Fatal for this key. QuotaExhausted, + + /// + /// No route to the provider. Fatal for this turn and explicitly NOT rotated on: when the + /// network is down every model fails identically, so rotating would burn the retry budget + /// and leave all models cooling down for a fault unrelated to any of them. + /// NetworkDown, + + /// Response could not be parsed. Retryable — the next model may answer cleanly. MalformedResponse, + + /// + /// The request itself is unacceptable (context overflow, unknown model id, bad parameters). + /// Fatal: retrying an identical request on another model cannot succeed, and doing so would + /// cool down every model in turn. + /// + InvalidRequest, } diff --git a/src/OpenKey.Core/Providers/ChatModels.cs b/src/OpenKey.Core/Providers/ChatModels.cs index 1e9ca49..b59b13f 100644 --- a/src/OpenKey.Core/Providers/ChatModels.cs +++ b/src/OpenKey.Core/Providers/ChatModels.cs @@ -1,3 +1,5 @@ +using System.Text.Json.Serialization; + namespace OpenKey.Core.Providers; public sealed record ChatRequest( @@ -6,16 +8,44 @@ public sealed record ChatRequest( int? MaxTokens = null, double? Temperature = null); -public sealed record ChatMessage(string Role, string Content, DateTimeOffset Timestamp) +public sealed record ChatMessage { + /// + /// Marked explicitly because the convenience overload below makes two constructors visible, and + /// System.Text.Json refuses to guess between them. Without this, deserializing session.json + /// threw NotSupportedException — which the store did not catch, so restoring a saved + /// conversation never worked at all. + /// + [JsonConstructor] + public ChatMessage(string role, string content, DateTimeOffset timestamp) + { + Role = role; + Content = content; + Timestamp = timestamp; + } + public ChatMessage(string role, string content) : this(role, content, DateTimeOffset.UtcNow) { } + public string Role { get; init; } + public string Content { get; init; } + public DateTimeOffset Timestamp { get; init; } + public const string SystemRole = "system"; public const string UserRole = "user"; public const string AssistantRole = "assistant"; } -public sealed record ChatChunk(string DeltaText, bool IsFinal, string? FinishReason); +/// +/// Set when the engine has abandoned a failed attempt and is starting over on another model. +/// Any text yielded before this point belongs to the discarded attempt: a consumer that is +/// accumulating deltas must clear its buffer, or a mid-reply rotation renders the answer twice +/// concatenated while the persisted session stores it once. Providers never set this. +/// +public sealed record ChatChunk( + string DeltaText, + bool IsFinal, + string? FinishReason, + bool IsAttemptRestart = false); public sealed record ModelInfo( string Id, diff --git a/src/OpenKey.Core/Storage/IConfigStore.cs b/src/OpenKey.Core/Storage/IConfigStore.cs new file mode 100644 index 0000000..fd70009 --- /dev/null +++ b/src/OpenKey.Core/Storage/IConfigStore.cs @@ -0,0 +1,47 @@ +namespace OpenKey.Core.Storage; + +/// +/// User preferences, persisted to config.json. Shape is normative — see +/// docs/05-persistence-and-reset.md. +/// +/// +/// Model ids to try first, in order. A single entry is how a pinned model is expressed; an empty +/// list means "let rotation choose", which is the default. +/// +/// Palette name: default, dark, light, or mono. +/// Upper bound on reply length requested from the model. +public sealed record OpenKeyConfig( + IReadOnlyList PreferredModels, + string Theme, + int MaxTokens) +{ + public const string DefaultTheme = "default"; + public const int DefaultMaxTokens = 2048; + + public static OpenKeyConfig Default { get; } = + new(Array.Empty(), DefaultTheme, DefaultMaxTokens); + + /// + /// The pinned model, or null when rotation is free to choose. A view over + /// , not a stored field — JsonIgnore keeps it out of the + /// file, whose shape is normative in docs/05-persistence-and-reset.md. + /// + [System.Text.Json.Serialization.JsonIgnore] + public string? PinnedModel => PreferredModels.Count > 0 ? PreferredModels[0] : null; + + public OpenKeyConfig WithPinnedModel(string? modelId) => + this with + { + PreferredModels = modelId is null ? Array.Empty() : new[] { modelId }, + }; +} + +public interface IConfigStore +{ + /// Current preferences. Never null — a missing or unreadable file yields defaults. + OpenKeyConfig Current { get; } + + OpenKeyConfig Load(); + + void Save(OpenKeyConfig config); +} diff --git a/src/OpenKey.Core/Storage/JsonConfigStore.cs b/src/OpenKey.Core/Storage/JsonConfigStore.cs new file mode 100644 index 0000000..e9f1ae5 --- /dev/null +++ b/src/OpenKey.Core/Storage/JsonConfigStore.cs @@ -0,0 +1,86 @@ +using System.Text.Json; +using OpenKey.Core.AppPaths; + +namespace OpenKey.Core.Storage; + +public sealed class JsonConfigStore : IConfigStore +{ + private readonly IAppPaths _paths; + private OpenKeyConfig? _cached; + + public JsonConfigStore(IAppPaths paths) => _paths = paths; + + public OpenKeyConfig Current => _cached ??= Load(); + + public OpenKeyConfig Load() + { + var path = _paths.ConfigFile; + if (!File.Exists(path)) + { + _cached = OpenKeyConfig.Default; + return _cached; + } + + try + { + using var stream = File.OpenRead(path); + var loaded = JsonSerializer.Deserialize(stream, OpenKeyJsonContext.Default.OpenKeyConfig); + _cached = Normalize(loaded); + } + catch (Exception ex) when (ex is JsonException or IOException or NotSupportedException) + { + // Preferences are not worth failing a launch over. Quarantine and carry on with + // defaults, matching how a corrupt session is handled. + try { File.Move(path, path + ".broken-" + DateTimeOffset.UtcNow.ToUnixTimeSeconds()); } + catch (Exception move) when (move is IOException or UnauthorizedAccessException) { } + _cached = OpenKeyConfig.Default; + } + + return _cached; + } + + public void Save(OpenKeyConfig config) + { + _cached = Normalize(config); + + // Best-effort, like every store except the key: losing a preference must never take down a + // working session. + try + { + _paths.EnsureRoot(); + var path = _paths.ConfigFile; + var tmp = path + ".tmp"; + using (var stream = File.Create(tmp)) + { + JsonSerializer.Serialize(stream, _cached, OpenKeyJsonContext.Default.OpenKeyConfig); + } + File.Move(tmp, path, overwrite: true); + } + catch (Exception ex) when (ex is IOException or UnauthorizedAccessException) + { + } + } + + /// + /// A hand-edited config is expected — docs tell users they may edit this file — so every field + /// is treated as untrusted rather than assumed well-formed. + /// + private static OpenKeyConfig Normalize(OpenKeyConfig? config) + { + if (config is null) return OpenKeyConfig.Default; + + var models = config.PreferredModels? + .Where(m => !string.IsNullOrWhiteSpace(m)) + .ToArray() ?? Array.Empty(); + + var theme = string.IsNullOrWhiteSpace(config.Theme) + ? OpenKeyConfig.DefaultTheme + : config.Theme.Trim().ToLowerInvariant(); + + var maxTokens = config.MaxTokens is > 0 and <= 200_000 + ? config.MaxTokens + : OpenKeyConfig.DefaultMaxTokens; + + return new OpenKeyConfig(models, theme, maxTokens); + } +} diff --git a/src/OpenKey.Core/Storage/JsonModelCatalog.cs b/src/OpenKey.Core/Storage/JsonModelCatalog.cs index 637e471..837b881 100644 --- a/src/OpenKey.Core/Storage/JsonModelCatalog.cs +++ b/src/OpenKey.Core/Storage/JsonModelCatalog.cs @@ -6,12 +6,6 @@ namespace OpenKey.Core.Storage; public sealed class JsonModelCatalog : IModelCatalog { - private static readonly JsonSerializerOptions JsonOpts = new() - { - WriteIndented = true, - PropertyNamingPolicy = JsonNamingPolicy.CamelCase, - }; - private static readonly TimeSpan CacheTtl = TimeSpan.FromHours(24); private readonly IAppPaths _paths; @@ -42,6 +36,17 @@ public async Task RefreshAsync(CancellationToken ct) { var models = await _provider.ListModelsAsync(ct); var free = models.Where(m => m.IsFree).ToList(); + + if (free.Count == 0) + { + // Never cache an empty list. Doing so pinned "no models" for the full 24h TTL, and + // since every launch then found a valid-but-empty cache, the app stayed broken until + // someone deleted %APPDATA%\OpenKey by hand. + throw new ChatException( + ChatErrorKind.TransientServer, + "OpenRouter didn't return any free models. This is usually temporary."); + } + var env = new CacheEnvelope(DateTimeOffset.UtcNow, free); _memCache = env; SaveToDisk(env); @@ -61,7 +66,7 @@ public void ClearCache() try { using var stream = File.OpenRead(path); - return JsonSerializer.Deserialize(stream, JsonOpts); + return JsonSerializer.Deserialize(stream, OpenKeyJsonContext.Default.CacheEnvelope); } catch (Exception ex) when (ex is JsonException or IOException) { @@ -71,15 +76,25 @@ public void ClearCache() private void SaveToDisk(CacheEnvelope env) { - _paths.EnsureRoot(); - var path = _paths.ModelsCacheFile; - var tmp = path + ".tmp"; - using (var stream = File.Create(tmp)) + // Best-effort: the cache is a speed optimisation, so a full disk or a read-only roaming + // profile must not take down a turn that already succeeded. + try + { + _paths.EnsureRoot(); + var path = _paths.ModelsCacheFile; + var tmp = path + ".tmp"; + using (var stream = File.Create(tmp)) + { + JsonSerializer.Serialize(stream, env, OpenKeyJsonContext.Default.CacheEnvelope); + } + File.Move(tmp, path, overwrite: true); + } + catch (Exception ex) when (ex is IOException or UnauthorizedAccessException) { - JsonSerializer.Serialize(stream, env, JsonOpts); + // Keep running on the in-memory cache. } - File.Move(tmp, path, overwrite: true); } - private sealed record CacheEnvelope(DateTimeOffset FetchedAt, IReadOnlyList Models); + // internal, not private: OpenKeyJsonContext must be able to name it. + internal sealed record CacheEnvelope(DateTimeOffset FetchedAt, IReadOnlyList Models); } diff --git a/src/OpenKey.Core/Storage/JsonSessionStore.cs b/src/OpenKey.Core/Storage/JsonSessionStore.cs index 7637386..126c4d7 100644 --- a/src/OpenKey.Core/Storage/JsonSessionStore.cs +++ b/src/OpenKey.Core/Storage/JsonSessionStore.cs @@ -5,12 +5,6 @@ namespace OpenKey.Core.Storage; public sealed class JsonSessionStore : ISessionStore { - private static readonly JsonSerializerOptions JsonOpts = new() - { - WriteIndented = true, - PropertyNamingPolicy = JsonNamingPolicy.CamelCase, - }; - private readonly IAppPaths _paths; public JsonSessionStore(IAppPaths paths) => _paths = paths; @@ -23,7 +17,7 @@ public sealed class JsonSessionStore : ISessionStore try { await using var stream = File.OpenRead(path); - return await JsonSerializer.DeserializeAsync(stream, JsonOpts, ct); + return await JsonSerializer.DeserializeAsync(stream, OpenKeyJsonContext.Default.SessionSnapshot, ct); } catch (Exception ex) when (ex is JsonException or IOException) { @@ -35,20 +29,35 @@ public sealed class JsonSessionStore : ISessionStore public async Task SaveAsync(SessionSnapshot snap, CancellationToken ct) { - _paths.EnsureRoot(); - var path = _paths.SessionFile; - var tmp = path + ".tmp"; - - await using (var stream = File.Create(tmp)) + // Best-effort. This runs immediately after a reply has been generated but before it is + // shown; an unguarded throw here loses the user a reply they already paid for, over a + // full disk or a read-only roaming profile. Losing history is the lesser failure. + try + { + _paths.EnsureRoot(); + var path = _paths.SessionFile; + var tmp = path + ".tmp"; + + await using (var stream = File.Create(tmp)) + { + await JsonSerializer.SerializeAsync(stream, snap, OpenKeyJsonContext.Default.SessionSnapshot, ct); + } + File.Move(tmp, path, overwrite: true); + } + catch (Exception ex) when (ex is IOException or UnauthorizedAccessException) { - await JsonSerializer.SerializeAsync(stream, snap, JsonOpts, ct); } - File.Move(tmp, path, overwrite: true); } public void Clear() { - var path = _paths.SessionFile; - if (File.Exists(path)) File.Delete(path); + try + { + var path = _paths.SessionFile; + if (File.Exists(path)) File.Delete(path); + } + catch (Exception ex) when (ex is IOException or UnauthorizedAccessException) + { + } } } diff --git a/src/OpenKey.Core/Storage/OpenKeyJsonContext.cs b/src/OpenKey.Core/Storage/OpenKeyJsonContext.cs new file mode 100644 index 0000000..9c84660 --- /dev/null +++ b/src/OpenKey.Core/Storage/OpenKeyJsonContext.cs @@ -0,0 +1,19 @@ +using System.Text.Json.Serialization; +using OpenKey.Core.Engine; + +namespace OpenKey.Core.Storage; + +/// +/// Source-generated serialization for everything OpenKey persists under %APPDATA%\OpenKey\. +/// Reflection-based serialization is neither trim- nor AOT-safe, and the shapes here are the +/// on-disk contract described in docs/05-persistence-and-reset.md — they change rarely and +/// deliberately, which is exactly the case source generation is for. +/// +[JsonSourceGenerationOptions( + WriteIndented = true, + PropertyNamingPolicy = JsonKnownNamingPolicy.CamelCase)] +[JsonSerializable(typeof(SessionSnapshot))] +[JsonSerializable(typeof(OpenKeyConfig))] +[JsonSerializable(typeof(JsonModelCatalog.CacheEnvelope))] +[JsonSerializable(typeof(RotationPolicy.StateEnvelope))] +internal sealed partial class OpenKeyJsonContext : JsonSerializerContext; diff --git a/src/OpenKey.Providers.OpenRouter/OpenKey.Providers.OpenRouter.csproj b/src/OpenKey.Providers.OpenRouter/OpenKey.Providers.OpenRouter.csproj index 32cf5ae..f518d1b 100644 --- a/src/OpenKey.Providers.OpenRouter/OpenKey.Providers.OpenRouter.csproj +++ b/src/OpenKey.Providers.OpenRouter/OpenKey.Providers.OpenRouter.csproj @@ -3,6 +3,7 @@ OpenKey.Providers.OpenRouter OpenKey.Providers.OpenRouter false + true diff --git a/src/OpenKey.Providers.OpenRouter/OpenRouterProvider.cs b/src/OpenKey.Providers.OpenRouter/OpenRouterProvider.cs index f483d32..f59822d 100644 --- a/src/OpenKey.Providers.OpenRouter/OpenRouterProvider.cs +++ b/src/OpenKey.Providers.OpenRouter/OpenRouterProvider.cs @@ -1,3 +1,4 @@ +using System.Globalization; using System.Net; using System.Net.Http.Headers; using System.Net.Http.Json; @@ -14,6 +15,14 @@ public sealed class OpenRouterProvider : IChatProvider private const string RefererHeader = "https://openkey.local"; private const string TitleHeader = "OpenKey"; + // Streaming needs per-read deadlines, not one deadline for the whole response. HttpClient.Timeout + // covers reading the body even under ResponseHeadersRead, so a single blanket value aborts long + // but perfectly healthy replies. These bound how long we wait for the *next* byte instead, which + // is what actually distinguishes a slow model from a dead connection. + private static readonly TimeSpan FirstTokenTimeout = TimeSpan.FromSeconds(45); + private static readonly TimeSpan StallTimeout = TimeSpan.FromSeconds(30); + private static readonly TimeSpan ModelListTimeout = TimeSpan.FromSeconds(30); + private static readonly JsonSerializerOptions JsonOpts = new() { PropertyNameCaseInsensitive = true, @@ -36,25 +45,59 @@ public async Task> ListModelsAsync(CancellationToken ct using var req = new HttpRequestMessage(HttpMethod.Get, $"{BaseUrl}/models"); ApplyHeaders(req); + // HttpClient.Timeout is infinite so it can't abort long streams; this call is not a stream, + // so it carries its own deadline. + using var listCts = CancellationTokenSource.CreateLinkedTokenSource(ct); + listCts.CancelAfter(ModelListTimeout); + HttpResponseMessage resp; try { - resp = await _http.SendAsync(req, HttpCompletionOption.ResponseHeadersRead, ct); + resp = await _http.SendAsync(req, HttpCompletionOption.ResponseHeadersRead, listCts.Token); + } + catch (OperationCanceledException) when (!ct.IsCancellationRequested) + { + throw new ChatException(ChatErrorKind.NetworkDown, "OpenRouter didn't respond in time."); } catch (Exception ex) when (IsNetwork(ex)) { throw new ChatException(ChatErrorKind.NetworkDown, "Network unreachable.", null, ex); } - await using var stream = await resp.Content.ReadAsStreamAsync(ct); + await using var stream = await resp.Content.ReadAsStreamAsync(listCts.Token); if (!resp.IsSuccessStatusCode) { - var body = await ReadBodyAsync(stream, ct); + var body = await ReadBodyAsync(stream, listCts.Token); throw MapHttpError(resp, body); } - using var doc = await JsonDocument.ParseAsync(stream, default, ct); - var data = doc.RootElement.GetProperty("data"); + // A captive portal (hotel, airport, café) answers any request with its own login page and + // an HTTP 200. Unguarded, that lands here as an unhandled JsonException during first run + // and takes the whole window down with it. + JsonDocument doc; + try + { + doc = await JsonDocument.ParseAsync(stream, default, listCts.Token); + } + catch (JsonException ex) + { + throw new ChatException( + ChatErrorKind.NetworkDown, + "Got a reply from the network, but it wasn't from OpenRouter. " + + "If you're on public Wi-Fi you may still need to sign in to it.", + null, + ex); + } + + using (doc) + { + if (!doc.RootElement.TryGetProperty("data", out var data) + || data.ValueKind != JsonValueKind.Array) + { + throw new ChatException( + ChatErrorKind.MalformedResponse, + "OpenRouter's model list came back in a format OpenKey doesn't recognise."); + } var list = new List(capacity: data.GetArrayLength()); foreach (var m in data.EnumerateArray()) @@ -74,31 +117,38 @@ public async Task> ListModelsAsync(CancellationToken ct } return list; + } } public async IAsyncEnumerable StreamChatAsync( ChatRequest request, [EnumeratorCancellation] CancellationToken ct) { - var body = new - { - model = request.Model, - messages = request.Messages.Select(m => new { role = m.Role, content = m.Content }).ToArray(), - stream = true, - max_tokens = request.MaxTokens ?? 2048, - temperature = request.Temperature, - }; + var body = new ChatCompletionRequest( + Model: request.Model, + Messages: request.Messages.Select(m => new WireMessage(m.Role, m.Content)).ToArray(), + Stream: true, + MaxTokens: request.MaxTokens ?? 2048, + Temperature: request.Temperature); using var req = new HttpRequestMessage(HttpMethod.Post, $"{BaseUrl}/chat/completions") { - Content = JsonContent.Create(body, options: JsonOpts), + Content = JsonContent.Create(body, OpenRouterJsonContext.Default.ChatCompletionRequest), }; ApplyHeaders(req); HttpResponseMessage resp; try { - resp = await _http.SendAsync(req, HttpCompletionOption.ResponseHeadersRead, ct); + // Deadline covers only getting response headers back. Once the stream is open the + // per-read deadlines below take over, so a slow-but-alive model is never cut off. + using var connectCts = CancellationTokenSource.CreateLinkedTokenSource(ct); + connectCts.CancelAfter(FirstTokenTimeout); + resp = await _http.SendAsync(req, HttpCompletionOption.ResponseHeadersRead, connectCts.Token); + } + catch (OperationCanceledException) when (!ct.IsCancellationRequested) + { + throw new ChatException(ChatErrorKind.TransientServer, "The model didn't accept the request in time."); } catch (Exception ex) when (IsNetwork(ex)) { @@ -115,18 +165,41 @@ public async IAsyncEnumerable StreamChatAsync( using var reader = new StreamReader(stream); + // Some models never emit a `finish_reason` chunk and simply close with [DONE]. Remember the + // last one seen so a complete reply isn't reported as unfinished, discarded, and retried. + string? lastFinishReason = null; + + var sawAnyData = false; + while (true) { string? line; + + // Bound the wait for the *next* line, not the whole response. + using var readCts = CancellationTokenSource.CreateLinkedTokenSource(ct); + readCts.CancelAfter(sawAnyData ? StallTimeout : FirstTokenTimeout); + try { - line = await reader.ReadLineAsync(ct); + line = await reader.ReadLineAsync(readCts.Token); + } + catch (OperationCanceledException) when (!ct.IsCancellationRequested) + { + // Our deadline fired, not the user's cancellation. Transient, so rotation moves on + // to a model that is actually producing output. + throw new ChatException( + ChatErrorKind.TransientServer, + sawAnyData + ? "The model stopped part-way through its reply." + : "The model didn't start replying in time."); } catch (Exception ex) when (IsNetwork(ex)) { throw new ChatException(ChatErrorKind.NetworkDown, "Connection lost mid-stream.", null, ex); } + sawAnyData = true; + if (line is null) break; if (line.Length == 0) continue; if (line.StartsWith(':')) continue; @@ -135,7 +208,10 @@ public async IAsyncEnumerable StreamChatAsync( var payload = line.Substring("data: ".Length); if (payload == "[DONE]") { - yield return new ChatChunk(string.Empty, IsFinal: true, FinishReason: null); + // "stop" is the correct default: the server closed the stream cleanly, which is + // exactly what a normal completion looks like. + yield return new ChatChunk( + string.Empty, IsFinal: true, FinishReason: lastFinishReason ?? "stop"); yield break; } @@ -150,6 +226,7 @@ public async IAsyncEnumerable StreamChatAsync( } if (parsed is null) continue; + if (parsed.FinishReason is not null) lastFinishReason = parsed.FinishReason; yield return parsed; if (parsed.IsFinal) yield break; } @@ -206,8 +283,14 @@ private static bool IsFreeModel(JsonElement model, string id) private static bool IsZero(JsonElement pricing, string field) { if (!pricing.TryGetProperty(field, out var v)) return false; + + // Parse rather than string-match: exact comparison against "0"/"0.0"/"0.00" silently + // classified a free model as paid the moment OpenRouter formatted it as "0.000000". + if (v.ValueKind == JsonValueKind.Number) return v.GetDouble() == 0d; + var s = v.ValueKind == JsonValueKind.String ? v.GetString() : v.ToString(); - return s == "0" || s == "0.0" || s == "0.00"; + return double.TryParse(s, NumberStyles.Float, CultureInfo.InvariantCulture, out var d) + && d == 0d; } private static async Task ReadBodyAsync(Stream stream, CancellationToken ct) @@ -224,13 +307,18 @@ private static ChatException MapHttpError(HttpResponseMessage resp, string body) var status = (int)resp.StatusCode; var kind = resp.StatusCode switch { - HttpStatusCode.Unauthorized => ChatErrorKind.AuthFailure, - HttpStatusCode.Forbidden => ChatErrorKind.AuthFailure, - HttpStatusCode.PaymentRequired => ChatErrorKind.QuotaExhausted, - HttpStatusCode.RequestTimeout => ChatErrorKind.TransientServer, - HttpStatusCode.TooManyRequests => ChatErrorKind.TransientRateLimit, - _ when status >= 500 && status < 600 => ChatErrorKind.TransientServer, - _ => ChatErrorKind.MalformedResponse, + HttpStatusCode.Unauthorized => ChatErrorKind.AuthFailure, + HttpStatusCode.Forbidden => ChatErrorKind.AuthFailure, + HttpStatusCode.PaymentRequired => ChatErrorKind.QuotaExhausted, + HttpStatusCode.RequestTimeout => ChatErrorKind.TransientServer, + HttpStatusCode.TooManyRequests => ChatErrorKind.TransientRateLimit, + // 400/404/422 mean the request is wrong (context overflow, unknown model). Retrying it + // unchanged on five other models cannot work and cools all of them down on the way. + HttpStatusCode.BadRequest => ChatErrorKind.InvalidRequest, + HttpStatusCode.NotFound => ChatErrorKind.InvalidRequest, + HttpStatusCode.UnprocessableEntity => ChatErrorKind.InvalidRequest, + _ when status >= 500 && status < 600 => ChatErrorKind.TransientServer, + _ => ChatErrorKind.MalformedResponse, }; return new ChatException(kind, $"HTTP {status}: {message}", retryAfter); diff --git a/src/OpenKey.Providers.OpenRouter/OpenRouterWire.cs b/src/OpenKey.Providers.OpenRouter/OpenRouterWire.cs new file mode 100644 index 0000000..d182259 --- /dev/null +++ b/src/OpenKey.Providers.OpenRouter/OpenRouterWire.cs @@ -0,0 +1,26 @@ +using System.Text.Json.Serialization; + +namespace OpenKey.Providers.OpenRouter; + +/// +/// Request bodies for the OpenRouter wire format documented in docs/03-openrouter-integration.md. +/// These are named types rather than anonymous objects so they can be source-generated: anonymous +/// types force reflection-based serialization, which is neither trim- nor AOT-safe. +/// +internal sealed record ChatCompletionRequest( + [property: JsonPropertyName("model")] string Model, + [property: JsonPropertyName("messages")] IReadOnlyList Messages, + [property: JsonPropertyName("stream")] bool Stream, + [property: JsonPropertyName("max_tokens")] int MaxTokens, + // Omitted entirely when unset. Previously serialized as `"temperature": null` on every request. + [property: JsonPropertyName("temperature")] + [property: JsonIgnore(Condition = JsonIgnoreCondition.WhenWritingNull)] + double? Temperature); + +internal sealed record WireMessage( + [property: JsonPropertyName("role")] string Role, + [property: JsonPropertyName("content")] string Content); + +[JsonSourceGenerationOptions(PropertyNameCaseInsensitive = true)] +[JsonSerializable(typeof(ChatCompletionRequest))] +internal sealed partial class OpenRouterJsonContext : JsonSerializerContext; diff --git a/src/OpenKey/CommandRouter.cs b/src/OpenKey/CommandRouter.cs index cec3204..403f88f 100644 --- a/src/OpenKey/CommandRouter.cs +++ b/src/OpenKey/CommandRouter.cs @@ -1,8 +1,11 @@ -using System.Reflection; +using System.Diagnostics; +using System.Globalization; +using System.Text; using OpenKey.Core.AppPaths; using OpenKey.Core.Engine; using OpenKey.Core.Providers; using OpenKey.Core.Storage; +using OpenKey.Ui; using Spectre.Console; namespace OpenKey; @@ -14,19 +17,25 @@ public sealed class CommandRouter private readonly ChatEngine _engine; private readonly IAppPaths _paths; private readonly IModelCatalog _catalog; + private readonly IConfigStore _config; private readonly Func _resetAction; private readonly Action _clearScreen; + /// Set when a command wants the host to resend a message — see /retry. + public string? PendingResend { get; private set; } + public CommandRouter( ChatEngine engine, IAppPaths paths, IModelCatalog catalog, + IConfigStore config, Func resetAction, Action clearScreen) { _engine = engine; _paths = paths; _catalog = catalog; + _config = config; _resetAction = resetAction; _clearScreen = clearScreen; } @@ -35,8 +44,11 @@ public async Task HandleAsync(string input, CancellationToken ct) { if (!input.StartsWith('/')) return CommandResult.NotACommand; + PendingResend = null; + var parts = input.Trim().Split(' ', 2); var cmd = parts[0].ToLowerInvariant(); + var arg = parts.Length > 1 ? parts[1].Trim() : null; switch (cmd) { @@ -48,6 +60,29 @@ public async Task HandleAsync(string input, CancellationToken ct) _clearScreen(); return CommandResult.Handled; + case "/new": + await StartNewConversationAsync(ct); + return CommandResult.Handled; + + case "/retry": + return Retry(); + + case "/history": + ShowHistory(); + return CommandResult.Handled; + + case "/export": + Export(arg); + return CommandResult.Handled; + + case "/copy": + CopyLastReply(); + return CommandResult.Handled; + + case "/theme": + SetTheme(arg); + return CommandResult.Handled; + case "/model": ShowActiveModel(); return CommandResult.Handled; @@ -65,29 +100,215 @@ public async Task HandleAsync(string input, CancellationToken ct) return CommandResult.Handled; case "/reset": - if (AnsiConsole.Confirm("Wipe all OpenKey data and start fresh?", defaultValue: false)) + // Destructive confirms always state exactly what is lost first, and never default + // to yes. + Components.StatusCard( + Severity.Warn, + "This erases everything", + "Your saved key and your entire chat history will be deleted from this PC. " + + "You'll need to sign in again.", + "Only continue if you meant to start completely fresh."); + + if (AnsiConsole.Confirm("Erase everything and start over?", defaultValue: false)) { await _resetAction(ct); } else { - AnsiConsole.MarkupLine("[grey]reset cancelled.[/]"); + Components.HintLine("Nothing was changed."); } return CommandResult.Handled; default: - AnsiConsole.MarkupLine($"[red]unknown command:[/] {Markup.Escape(cmd)}"); + AnsiConsole.MarkupLine($"[{Theme.Muted}]There's no[/] {Markup.Escape(cmd)} [{Theme.Muted}]command.[/]"); + Components.HintLine("Type /help to see what OpenKey can do."); return CommandResult.Handled; } } + /// + /// Clears the conversation but keeps the key. Previously the only way to start fresh was + /// /reset, which also deleted the key and forced a new sign-in. + /// + private async Task StartNewConversationAsync(CancellationToken ct) + { + if (!_engine.Turns.Any(t => t.Role != ChatMessage.SystemRole)) + { + Components.HintLine("Already a fresh conversation."); + return; + } + + await _engine.NewSessionAsync(ct); + _clearScreen(); + Components.SuccessLine("Started a new conversation. Your key is untouched."); + } + + private CommandResult Retry() + { + if (_engine.LastUserMessage is not { } last) + { + Components.HintLine("Nothing to retry yet — send a message first."); + return CommandResult.Handled; + } + + PendingResend = last; + Components.HintLine($"Resending: {Markup.Escape(Shorten(last, 60))}"); + return CommandResult.Handled; + } + + private void ShowHistory() + { + var turns = _engine.Turns.Where(t => t.Role != ChatMessage.SystemRole).ToList(); + if (turns.Count == 0) + { + Components.HintLine("No messages yet."); + return; + } + + var table = new Table() + .Border(Glyphs.Table) + .BorderColor(Color.Grey) + .Expand() + .AddColumn(new TableColumn($"[{Theme.Strong}]Who[/]").Width(12)) + .AddColumn(new TableColumn($"[{Theme.Strong}]Message[/]")); + + foreach (var t in turns) + { + var who = t.Role == ChatMessage.UserRole ? Environment.UserName : "OpenKey AI"; + var style = t.Role == ChatMessage.UserRole ? Theme.Strong : Theme.Brand; + table.AddRow( + $"[{style}]{Markup.Escape(who)}[/]", + Markup.Escape(Shorten(t.Content.ReplaceLineEndings(" ").Trim(), 400))); + } + + AnsiConsole.Write(table); + AnsiConsole.WriteLine(); + Components.HintLine($"{turns.Count} messages. Use /export to save the full text."); + } + + private void Export(string? path) + { + var turns = _engine.Turns.Where(t => t.Role != ChatMessage.SystemRole).ToList(); + if (turns.Count == 0) + { + Components.HintLine("Nothing to export yet."); + return; + } + + // Default to a timestamped file on the Desktop: a non-technical user shouldn't have to + // think about paths, and a bare /export should still do something obviously useful. + if (string.IsNullOrWhiteSpace(path)) + { + var desktop = Environment.GetFolderPath(Environment.SpecialFolder.DesktopDirectory); + var stamp = DateTimeOffset.Now.ToString("yyyy-MM-dd-HHmm", CultureInfo.InvariantCulture); + path = Path.Combine(desktop, $"OpenKey-chat-{stamp}.md"); + } + + try + { + var full = Path.GetFullPath(path); + var dir = Path.GetDirectoryName(full); + if (!string.IsNullOrEmpty(dir)) Directory.CreateDirectory(dir); + + var sb = new StringBuilder(); + sb.Append("# OpenKey conversation\n\n"); + sb.Append(CultureInfo.InvariantCulture, $"Exported {DateTimeOffset.Now:yyyy-MM-dd HH:mm}\n"); + if (_engine.ActiveModel is { } m) + sb.Append(CultureInfo.InvariantCulture, $"Model: {m.Id}\n"); + sb.Append('\n'); + + foreach (var t in turns) + { + var who = t.Role == ChatMessage.UserRole ? "You" : "OpenKey AI"; + sb.Append(CultureInfo.InvariantCulture, $"## {who}\n\n{t.Content.TrimEnd()}\n\n"); + } + + File.WriteAllText(full, sb.ToString()); + Components.SuccessLine($"Saved to {full}"); + } + catch (Exception ex) when (ex is IOException or UnauthorizedAccessException or ArgumentException or NotSupportedException) + { + Components.StatusCard( + Severity.Warn, + "Couldn't save the file", + ex.Message, + "Try /export with a different location, for example /export C:\\Users\\Me\\chat.md"); + } + } + + /// + /// Copies the last reply via clip.exe. A console app has no clipboard API without + /// dragging in a UI framework, and clip.exe ships with Windows. + /// + private void CopyLastReply() + { + var last = _engine.Turns.LastOrDefault(t => t.Role == ChatMessage.AssistantRole); + if (last is null) + { + Components.HintLine("No reply to copy yet."); + return; + } + + try + { + var psi = new ProcessStartInfo("clip.exe") + { + RedirectStandardInput = true, + UseShellExecute = false, + CreateNoWindow = true, + }; + using var proc = Process.Start(psi); + if (proc is null) + { + Components.HintLine("Couldn't reach the Windows clipboard."); + return; + } + + proc.StandardInput.Write(last.Content); + proc.StandardInput.Close(); + proc.WaitForExit(5000); + + Components.SuccessLine("Last reply copied to the clipboard."); + } + catch (Exception ex) when (ex is System.ComponentModel.Win32Exception or IOException or InvalidOperationException) + { + Components.HintLine("Couldn't reach the Windows clipboard."); + } + } + + private void SetTheme(string? name) + { + if (string.IsNullOrWhiteSpace(name)) + { + AnsiConsole.MarkupLine( + $"Current theme: [{Theme.Brand}]{Markup.Escape(Theme.Current)}[/]"); + Components.HintLine($"Choose one of: {string.Join(", ", OpenKeyConfigThemes.All)} — for example /theme light"); + return; + } + + var wanted = name.Trim().ToLowerInvariant(); + if (!Theme.IsKnown(wanted)) + { + Components.HintLine($"There's no \"{Markup.Escape(wanted)}\" theme. Try: {string.Join(", ", OpenKeyConfigThemes.All)}"); + return; + } + + Theme.Apply(wanted); + _config.Save(_config.Current with { Theme = wanted }); + _clearScreen(); + Components.SuccessLine($"Theme set to {wanted}."); + } + + private static string Shorten(string value, int max) => + value.Length <= max ? value : value[..(max - 1)] + Glyphs.Ellipsis; + private void ShowActiveModel() { var m = _engine.ActiveModel; if (m is null) - AnsiConsole.MarkupLine("[grey]no active model yet — send a message first.[/]"); + Components.HintLine("No model has answered yet. Send a message first."); else - AnsiConsole.MarkupLine($"current model: [bold cyan]{Markup.Escape(m.Id)}[/]"); + AnsiConsole.MarkupLine($"Currently answering with [{Theme.Brand}]{Markup.Escape(m.Id)}[/]."); } private async Task ShowModelPickerAsync(CancellationToken ct) @@ -96,88 +317,118 @@ private async Task ShowModelPickerAsync(CancellationToken ct) try { await AnsiConsole.Status() - .Spinner(Spinner.Known.Dots) - .StartAsync("loading free models…", async _ => + .Spinner(Glyphs.Spinner) + .SpinnerStyle(new Style(Color.Grey)) + .StartAsync($"[{Theme.Muted}]Loading models[/]", async _ => { models = await _catalog.GetFreeModelsAsync(ct); }); } catch (ChatException ex) { - AnsiConsole.MarkupLine($"[red]could not load models ({ex.Kind}):[/] {Markup.Escape(ex.Message)}"); + Components.StatusCard( + Severity.Warn, + "Couldn't load the model list", + ex.Message, + "Check your connection and try /models again."); return; } if (models is null || models.Count == 0) { - AnsiConsole.MarkupLine("[red]no free models available.[/]"); + Components.StatusCard( + Severity.Warn, + "No free models available", + "OpenRouter didn't offer any free models just now.", + "This is usually temporary. Try /models again shortly."); return; } - var prompt = new SelectionPrompt() - .Title("pick a model ([grey]↑/↓ to scroll, enter to select[/])") - .PageSize(15) - .MoreChoicesText("[grey](move up and down for more)[/]") - .AddChoices(new[] { AutoChoiceLabel }.Concat(models.Select(m => m.Id))); + // Show the human-readable name and context size, not the raw id. DisplayName was fetched + // from the API and then never displayed anywhere. + var rows = models + .Select(m => $"{Truncate(m.DisplayName, 44).PadRight(46)}{FormatContext(m.ContextLength)}") + .ToList(); + + var prompt = new SelectionPrompt + { + Title = Components.PickerTitle("Which model should answer you?"), + PageSize = 12, + MoreChoicesText = $"[{Theme.Muted}]More below[/]", + }; + prompt.AddChoice(AutoChoiceLabel); + foreach (var row in rows) prompt.AddChoice(row); - var choice = AnsiConsole.Prompt(prompt); + string choice = AnsiConsole.Prompt(prompt); if (choice == AutoChoiceLabel) { _engine.PreferredModelId = null; - AnsiConsole.MarkupLine("[green]rotation re-enabled.[/]"); + Components.SuccessLine("OpenKey will pick the best available model for each message."); return; } - _engine.PreferredModelId = choice; - var ctx = models.FirstOrDefault(m => m.Id == choice)?.ContextLength; - var ctxText = ctx is > 0 ? $" [grey](ctx {ctx})[/]" : string.Empty; - AnsiConsole.MarkupLine($"[green]pinned:[/] [bold cyan]{Markup.Escape(choice)}[/]{ctxText} [grey](until restart)[/]"); + var picked = models[rows.IndexOf(choice)]; + _engine.PreferredModelId = picked.Id; + Components.SuccessLine($"Now using {picked.DisplayName}."); + Components.HintLine("This lasts until you close OpenKey."); } - private void ShowAbout() - { - var ver = typeof(CommandRouter).Assembly - .GetCustomAttribute()?.InformationalVersion - ?? typeof(CommandRouter).Assembly.GetName().Version?.ToString() - ?? "dev"; - - var active = _engine.ActiveModel?.Id ?? "(none yet — send a message)"; - var pinned = _engine.PreferredModelId ?? "(auto / rotate)"; - - var grid = new Grid() - .AddColumn(new GridColumn().NoWrap().PadRight(2)) - .AddColumn(); - - grid.AddRow("[grey]Version[/]", $"[bold]{Markup.Escape(ver)}[/]"); - grid.AddRow("[grey]Data dir[/]", Markup.Escape(_paths.RootDir)); - grid.AddRow("[grey]Active model[/]", Markup.Escape(active)); - grid.AddRow("[grey]Pinned model[/]", Markup.Escape(pinned)); - grid.AddRow("[grey]Developer[/]", "Paolo Patron"); - - AnsiConsole.Write(new Panel(grid) - .Header("[bold cyan] OpenKey [/]") - .Border(BoxBorder.Rounded) - .BorderColor(Color.Grey)); - } + private static string FormatContext(int contextLength) => + contextLength <= 0 ? string.Empty + : contextLength >= 1000 ? $"{contextLength / 1000}k context" + : $"{contextLength} context"; + + private static string Truncate(string value, int max) => + value.Length <= max ? value : value[..(max - 1)] + Glyphs.Ellipsis; + + private void ShowAbout() => + Components.KeyValuePanel("About OpenKey", new (string, string)[] + { + ("Version", Components.Version), + ("Answering with", _engine.ActiveModel?.Id ?? "Nothing yet — send a message"), + ("Model choice", _engine.PreferredModelId ?? "Automatic"), + ("Your data", _paths.RootDir), + ("Key security", "Encrypted for your Windows account, stored on this PC only"), + ("Developer", "Paolo Patron"), + }); private static void ShowHelp() { var table = new Table() - .Border(TableBorder.Rounded) + .Border(Glyphs.Table) .BorderColor(Color.Grey) - .AddColumn(new TableColumn("[bold]Command[/]")) - .AddColumn(new TableColumn("[bold]What it does[/]")); + .Expand() + .AddColumn(new TableColumn($"[{Theme.Strong}]Command[/]")) + .AddColumn(new TableColumn($"[{Theme.Strong}]What it does[/]")); + + void Row(string cmd, string what) => + table.AddRow($"[{Theme.Brand}]{cmd}[/]", what); + + table.AddRow($"[{Theme.Muted}]Chatting[/]", string.Empty); + Row("/new", "Start a fresh conversation, keeping your key"); + Row("/retry", "Send your last message again"); + Row("/history", "Show the conversation so far"); + Row("/copy", "Copy the last reply to the clipboard"); + Row("/export", "Save the conversation as a markdown file"); + + table.AddEmptyRow(); + table.AddRow($"[{Theme.Muted}]Models[/]", string.Empty); + Row("/models", "Choose which AI model answers you"); + Row("/model", "Show which model is answering right now"); - table.AddRow("[cyan]/about[/]", "Show version, data dir, active model, dev info"); - table.AddRow("[cyan]/models[/]", "Pick a free model with arrow keys ([grey]pinned until restart[/])"); - table.AddRow("[cyan]/model[/]", "Show the current active free model"); - table.AddRow("[cyan]/cls[/]", "Clear the screen and reprint the header"); - table.AddRow("[cyan]/help[/]", "Show this list of commands"); - table.AddRow("[cyan]/reset[/]", "Wipe all OpenKey data and re-run first-run setup"); - table.AddRow("[cyan]/quit[/]", "Exit the app cleanly ([grey]alias: /exit[/])"); + table.AddEmptyRow(); + table.AddRow($"[{Theme.Muted}]OpenKey[/]", string.Empty); + Row("/theme", "Switch colours: default, dark, light, mono"); + Row("/about", "Show version, where your data lives, and who made this"); + Row("/cls", "Clear the screen"); + Row("/help", "Show this list"); + Row("/reset", "Erase everything and start over, including your key and chat history"); + Row("/quit", "Close OpenKey"); AnsiConsole.Write(table); + AnsiConsole.WriteLine(); + Components.HintLine("Anything that doesn't start with / is sent to the AI. Press Ctrl+C to stop a reply."); } } diff --git a/src/OpenKey/ConsoleHost.cs b/src/OpenKey/ConsoleHost.cs index bc342dd..a54b047 100644 --- a/src/OpenKey/ConsoleHost.cs +++ b/src/OpenKey/ConsoleHost.cs @@ -6,16 +6,17 @@ using OpenKey.Core.Storage; using OpenKey.OAuth; using OpenKey.Providers.OpenRouter; +using OpenKey.Ui; using Spectre.Console; namespace OpenKey; public sealed class ConsoleHost { - private const string AiLabel = "[bold magenta]OpenKey AI[/]"; - - private readonly string _userPrompt = $"[bold cyan]{Markup.Escape(Environment.UserName)}[/]: "; - private readonly string _userLabel = $"[bold cyan]{Markup.Escape(Environment.UserName)}[/]"; + // Computed, not a field initializer: those run at DI construction, before ConsoleLayout.Initialize + // resolves the glyph tier, so a cached value would always be the ASCII fallback. + private static string UserPrompt => + $"[{Theme.Strong}]{Markup.Escape(Environment.UserName)}[/] [{Theme.Brand}]{Glyphs.Caret}[/] "; private readonly IAppPaths _paths; private readonly IKeyStore _keyStore; @@ -23,10 +24,17 @@ public sealed class ConsoleHost private readonly IModelCatalog _catalog; private readonly IRotationPolicy _rotation; private readonly ChatEngine _engine; + private readonly IConfigStore _config; private readonly HttpClient _http; private CommandRouter _commands = default!; + private CancellationTokenSource? _turnCts; + private volatile bool _exiting; + + /// Shown once per session — a hint repeated on every turn becomes noise. + private bool _shownStopHint; + public ConsoleHost( IAppPaths paths, IKeyStore keyStore, @@ -34,6 +42,7 @@ public ConsoleHost( IModelCatalog catalog, IRotationPolicy rotation, ChatEngine engine, + IConfigStore config, HttpClient http) { _paths = paths; @@ -42,151 +51,256 @@ public ConsoleHost( _catalog = catalog; _rotation = rotation; _engine = engine; + _config = config; _http = http; } public async Task RunAsync() { - using var cts = new CancellationTokenSource(); + // One CTS per turn, held here so the Ctrl+C handler can reach the in-flight one. + // The previous design used a single process-lifetime CTS: once Ctrl+C cancelled it, every + // later turn was born already cancelled and the session was unusable until restart. Console.CancelKeyPress += (_, e) => { - e.Cancel = true; - cts.Cancel(); + e.Cancel = true; // always swallow; never let the CLR kill us + var turn = Volatile.Read(ref _turnCts); + if (turn is not null && !turn.IsCancellationRequested) + { + turn.Cancel(); // mid-turn: cancel just this reply + } + else + { + _exiting = true; // at the prompt: quit + } }; - PrintBanner(); + ConsoleLayout.Initialize(); + Theme.Apply(_config.Current.Theme); + // The banner is printed by first-run setup (which needs it) or by the home header (which + // clears first). Printing it here too showed it twice whenever the screen couldn't be cleared. if (!await EnsureFirstRunAsync(CancellationToken.None)) { - AnsiConsole.MarkupLine("[red]setup aborted. exiting.[/]"); + Components.HintLine("Setup didn't finish, so OpenKey will close."); + HoldIfOwnConsole(); return; } - _commands = new CommandRouter(_engine, _paths, _catalog, ResetAllAsync, ClearAndShowChatHeader); - _engine.OnRotation += msg => AnsiConsole.MarkupLine($"[yellow]rotating: {Markup.Escape(msg)}[/]"); + _commands = new CommandRouter(_engine, _paths, _catalog, _config, ResetAllAsync, ClearAndShowChatHeader); ClearAndShowChatHeader(); await _engine.ResumeAsync(CancellationToken.None); ShowResumeRecapIfAny(); - while (true) + while (!_exiting) { - string line; + string? line; try { - line = AnsiConsole.Prompt(new TextPrompt(_userPrompt).AllowEmpty()); + line = ReadUserLine(); } catch (Exception) { break; } + if (line is null) break; // EOF / Ctrl+D + if (_exiting) break; if (string.IsNullOrWhiteSpace(line)) continue; var result = await _commands.HandleAsync(line, CancellationToken.None); if (result == CommandResult.Exit) break; - if (result == CommandResult.Handled) continue; + if (result == CommandResult.Handled) + { + // /retry asks the host to resend rather than sending from the router, so that + // resent messages take exactly the same path as typed ones. + if (_commands.PendingResend is { } resend) await SendAndRenderAsync(resend); + continue; + } - await SendAndRenderAsync(line, cts); + await SendAndRenderAsync(line); } - AnsiConsole.MarkupLine("[grey]goodbye.[/]"); + AnsiConsole.WriteLine(); + Components.HintLine("Thanks for using OpenKey."); + HoldIfOwnConsole(); } - private async Task SendAndRenderAsync(string userText, CancellationTokenSource outerCts) + private async Task SendAndRenderAsync(string userText) { - using var turnCts = CancellationTokenSource.CreateLinkedTokenSource(outerCts.Token); + var turnCts = new CancellationTokenSource(); + Volatile.Write(ref _turnCts, turnCts); var ct = turnCts.Token; + var started = System.Diagnostics.Stopwatch.StartNew(); + var rotations = 0; + void CountRotation(string _) => rotations++; + _engine.OnRotation += CountRotation; + try { await using var iter = _engine.SendAsync(userText, ct).GetAsyncEnumerator(ct); - var sb = new StringBuilder(); + // Phase 1 — spinner owns the screen until there is something to show. The enumerator is + // created outside the callback so it survives the handoff; returning ends the spinner. + ChatChunk? firstText = null; + var finishedDuringSpinner = false; + + // The backlog asked for a /stop command. A command can't work here — while a reply is + // streaming the app isn't reading a prompt — and Ctrl+C already cancels correctly. The + // real gap was that nothing said so, so the fix is discoverability, not a new verb. + // A key-watcher was considered and rejected: it would swallow type-ahead, and users + // routinely start composing their next message while a reply arrives. + var stopHint = _shownStopHint ? string.Empty : $" [{Theme.Muted}](Ctrl+C to stop)[/]"; + _shownStopHint = true; + await AnsiConsole.Status() - .Spinner(Spinner.Known.Dots) - .StartAsync($"{AiLabel} is thinking…", async _ => + .Spinner(Glyphs.Spinner) + .SpinnerStyle(new Style(Color.Grey)) + .StartAsync($"[{Theme.Muted}]Thinking[/]{stopHint}", async _ => { while (await iter.MoveNextAsync()) { var c = iter.Current; - if (!string.IsNullOrEmpty(c.DeltaText)) - sb.Append(c.DeltaText); - if (c.IsFinal) break; + if (!string.IsNullOrEmpty(c.DeltaText)) { firstText = c; return; } + if (c.IsFinal) { finishedDuringSpinner = true; return; } } + finishedDuringSpinner = true; }); - if (sb.Length == 0) + if (firstText is null && finishedDuringSpinner) { - AnsiConsole.MarkupLine("[grey](no response)[/]"); + Components.StatusCard( + Severity.Warn, + "No reply came back", + "The model accepted the message but returned nothing.", + "Send it again, or type /models to try a different model."); return; } - AnsiConsole.Markup($"{AiLabel}: "); - MarkdownConsoleRenderer.Render(AnsiConsole.Console, sb.ToString()); - Console.Out.Flush(); - } - catch (OperationCanceledException) - { + // Phase 2 — spinner is torn down and the cursor restored, so text can stream freely. + // Per-token writing under a live spinner garbles: the spinner repaints from column 0. + Components.ReplyHeader(_engine.ActiveModel?.Id ?? "unknown", started.Elapsed); + Components.RotationNote(rotations); + + var writer = new TranscriptWriter(AnsiConsole.Console); + writer.Append(firstText!.DeltaText); + + if (!firstText.IsFinal) + { + while (await iter.MoveNextAsync()) + { + var c = iter.Current; + + // The engine abandoned this attempt; everything shown so far belongs to it. + if (c.IsAttemptRestart) { writer.Reset(); rotations++; continue; } + + if (!string.IsNullOrEmpty(c.DeltaText)) writer.Append(c.DeltaText); + if (c.IsFinal) break; + } + } + + writer.Complete(); AnsiConsole.WriteLine(); - AnsiConsole.MarkupLine("[grey](cancelled)[/]"); } - catch (ChatException ex) when (ex.Kind == ChatErrorKind.AuthFailure) + catch (OperationCanceledException) when (turnCts.IsCancellationRequested) { + // Filtered on our own token so an upstream deadline isn't mislabelled "cancelled". + AnsiConsole.WriteLine(); + Components.HintLine("Stopped."); AnsiConsole.WriteLine(); - AnsiConsole.MarkupLine($"[red]auth failure:[/] {Markup.Escape(ex.Message)}"); - AnsiConsole.MarkupLine("[red]run [/]/reset[red] to re-enter your API key.[/]"); } catch (ChatException ex) { - AnsiConsole.WriteLine(); - AnsiConsole.MarkupLine($"[red]error ({ex.Kind}):[/] {Markup.Escape(ex.Message)}"); + ShowChatError(ex); + } + finally + { + _engine.OnRotation -= CountRotation; + Volatile.Write(ref _turnCts, null); + turnCts.Dispose(); + + // Status() hides the cursor, so an aborted turn must restore it — but only when there + // is a real console. Redirected, the legacy backend reaches for a handle that isn't + // there and throws "The handle is invalid", killing the app after a successful reply. + if (ConsoleLayout.Rich) + { + try { AnsiConsole.Cursor.Show(); } catch (IOException) { } + } } } - private static void PrintBanner() + /// + /// Maps an error onto copy the user can act on. Raw names and raw + /// exception messages never reach the screen — they were the most developer-tool-looking thing + /// in the app, and they told the user nothing about what to do next. + /// + private static void ShowChatError(ChatException ex) { - var ver = typeof(ConsoleHost).Assembly - .GetCustomAttribute()?.InformationalVersion - ?? typeof(ConsoleHost).Assembly.GetName().Version?.ToString() - ?? "dev"; - AnsiConsole.Write(new Rule($"[bold cyan]OpenKey[/] [grey]v{Markup.Escape(ver)}[/]").LeftJustified()); - AnsiConsole.MarkupLine("[grey]Developed by Paolo Patron[/]"); - AnsiConsole.WriteLine(); - } + var (severity, title, detail, next) = ex.Kind switch + { + ChatErrorKind.AuthFailure => ( + Severity.Danger, + "Your key was refused", + "OpenRouter did not accept the saved key. It may have been revoked or replaced.", + $"Type [{Theme.Brand}]/reset[/] to sign in again. This also erases your chat history."), + + ChatErrorKind.QuotaExhausted => ( + Severity.Danger, + "This key is out of credit", + "OpenRouter reports no remaining allowance for this key.", + "Add credit at https://openrouter.ai, or wait for your free allowance to renew."), + + ChatErrorKind.NetworkDown => ( + Severity.Warn, + "Can't reach OpenRouter", + ex.Message, + "Check your internet connection and send the message again."), + + ChatErrorKind.TransientRateLimit => ( + Severity.Warn, + "Every free model is busy right now", + ex.Message, + $"Wait a moment and send your message again, or type [{Theme.Brand}]/models[/] to pick a different one."), + + ChatErrorKind.InvalidRequest => ( + Severity.Danger, + "The model refused this message", + "It may be too long for the model's context, or the model may no longer exist.", + $"Try a shorter message, or type [{Theme.Brand}]/models[/] to pick a different model."), + + _ => ( + Severity.Warn, + "That didn't go through", + ex.Message, + $"Send it again, or type [{Theme.Brand}]/models[/] to try a different model."), + }; - private static void ClearAndShowChatHeader() - { - AnsiConsole.Clear(); - PrintBanner(); - - var body = new Markup( - "Type a message to start chatting.\n" + - "\n" + - "[cyan]/models[/] Choose a model\n" + - "[cyan]/help[/] View all commands\n" + - "[cyan]/quit[/] Exit"); - AnsiConsole.Write(new Panel(body) - .Header("[bold cyan] Getting started [/]") - .Border(BoxBorder.Rounded) - .BorderColor(Color.Grey) - .Padding(1, 0)); - AnsiConsole.WriteLine(); + Components.StatusCard(severity, title, detail, next); } + private static void ClearAndShowChatHeader() => Components.HomeHeader(); + + /// + /// Past turns are rendered entirely grey and indented, so the resumed history reads as inert + /// rather than as part of the live conversation. + /// private void ShowResumeRecapIfAny() { - var turns = _engine.Turns; - var nonSystem = turns.Where(t => t.Role != ChatMessage.SystemRole).ToList(); + var nonSystem = _engine.Turns.Where(t => t.Role != ChatMessage.SystemRole).ToList(); if (nonSystem.Count == 0) return; - AnsiConsole.MarkupLine("[grey]resumed previous session. last turns:[/]"); + Components.HintLine("Picking up where you left off."); + AnsiConsole.WriteLine(); + foreach (var t in nonSystem.TakeLast(2)) { - var label = t.Role == ChatMessage.UserRole ? _userLabel : AiLabel; - var preview = t.Content.Length > 200 ? t.Content[..200] + "…" : t.Content; - AnsiConsole.MarkupLine($"{label}: {Markup.Escape(preview)}"); + var label = t.Role == ChatMessage.UserRole ? Environment.UserName : "OpenKey AI"; + var flat = t.Content.ReplaceLineEndings(" ").Trim(); + var preview = flat.Length > 70 ? flat[..70] + Glyphs.Ellipsis : flat; + AnsiConsole.MarkupLine( + $" [{Theme.Muted}]{Markup.Escape(label.PadRight(12))} {Markup.Escape(preview)}[/]"); } AnsiConsole.WriteLine(); } @@ -200,18 +314,27 @@ private async Task EnsureFirstRunAsync(CancellationToken ct) _keyStore.Clear(); } - AnsiConsole.MarkupLine("[bold]welcome to OpenKey.[/]"); - AnsiConsole.MarkupLine("[grey]your key will be encrypted via Windows DPAPI for your user account only.[/]"); + Components.Banner(); + + // No acronyms on the first screen a new user sees. "DPAPI" was the product's second + // sentence; what matters to them is that the key stays on this PC. + AnsiConsole.MarkupLine("Welcome to OpenKey. Chat with capable AI models for free, with no subscription."); + AnsiConsole.WriteLine(); + AnsiConsole.MarkupLine( + "You need a free OpenRouter key once. OpenKey encrypts it for your Windows account and keeps it"); + AnsiConsole.MarkupLine("on this PC. It is never sent anywhere except OpenRouter."); AnsiConsole.WriteLine(); - const string OAuthChoice = "Sign in with browser (OAuth/PKCE) — recommended"; - const string PasteChoice = "I already have a key — paste it"; + const string OAuthChoice = "Sign in with my browser"; + const string PasteChoice = "Paste a key I already have"; for (int attempt = 1; attempt <= 3; attempt++) { + // No "(attempt 1/3)" counter: showing a retry budget before anything has failed + // manufactures anxiety. Retries are surfaced only after a failure. var choice = AnsiConsole.Prompt( new SelectionPrompt() - .Title($"how do you want to provide your OpenRouter key? (attempt {attempt}/3)") + .Title(Components.PickerTitle("How would you like to connect?")) .AddChoices(OAuthChoice, PasteChoice)); string? key = null; @@ -225,13 +348,17 @@ private async Task EnsureFirstRunAsync(CancellationToken ct) } catch (OperationCanceledException) { - AnsiConsole.MarkupLine("[grey](cancelled)[/]"); + Components.HintLine("Sign-in cancelled."); continue; } - catch (OAuthPortInUseException ex) + catch (OAuthPortInUseException) { - AnsiConsole.MarkupLine($"[yellow]⚠ {Markup.Escape(ex.Message)}[/]"); - AnsiConsole.MarkupLine("[grey]switching to paste in this attempt…[/]"); + Components.StatusCard( + Severity.Warn, + "Browser sign-in isn't available right now", + "OpenKey needs one of a few local ports for a moment to receive the sign-in, " + + "and every one of them is already in use.", + "Paste a key instead, or close the other app and try again."); string? pasteKey; try { pasteKey = AcquireKeyViaPaste(); } catch (Exception) { continue; } @@ -241,7 +368,11 @@ private async Task EnsureFirstRunAsync(CancellationToken ct) } catch (ChatException ex) { - AnsiConsole.MarkupLine($"[red]✗ {Markup.Escape(ex.Kind.ToString())}:[/] {Markup.Escape(ex.Message)}"); + Components.StatusCard( + Severity.Warn, + "Sign-in didn't complete", + ex.Message, + "Try again, or choose to paste a key instead."); continue; } @@ -251,13 +382,12 @@ private async Task EnsureFirstRunAsync(CancellationToken ct) key = outcome.Key; break; case OAuthOutcomeKind.UserChosePaste: - AnsiConsole.MarkupLine("[grey]switching to paste…[/]"); try { key = AcquireKeyViaPaste(); } catch (Exception) { continue; } break; case OAuthOutcomeKind.UserCanceled: default: - AnsiConsole.MarkupLine("[grey](cancelled)[/]"); + Components.HintLine("Sign-in cancelled."); continue; } } @@ -269,7 +399,7 @@ private async Task EnsureFirstRunAsync(CancellationToken ct) if (string.IsNullOrEmpty(key)) { - AnsiConsole.MarkupLine("[red]✗ no key acquired[/]"); + Components.HintLine("No key was entered."); continue; } @@ -281,8 +411,10 @@ private async Task EnsureFirstRunAsync(CancellationToken ct) private async Task AcquireKeyViaOAuthAsync(CancellationToken ct) { - AnsiConsole.MarkupLine("[grey]opening browser for OpenRouter sign-in…[/]"); - AnsiConsole.MarkupLine("[grey]press [/][bold]p[/][grey] to paste a key instead, [/][bold]c[/][grey] (or Esc) to cancel.[/]"); + AnsiConsole.WriteLine(); + AnsiConsole.MarkupLine("Opening your browser to sign in to OpenRouter."); + AnsiConsole.WriteLine(); + Components.HintLine("Press P to paste a key instead, or Esc to cancel."); using var interruptCts = CancellationTokenSource.CreateLinkedTokenSource(ct); var oauth = new OpenRouterOAuth(_http); @@ -334,12 +466,16 @@ private async Task AcquireKeyViaOAuthAsync(CancellationToken ct) private static string AcquireKeyViaPaste() { - AnsiConsole.MarkupLine("[grey]paste your OpenRouter API key (input is hidden). get one at [/][link]https://openrouter.ai/keys[/]"); + AnsiConsole.WriteLine(); + AnsiConsole.MarkupLine("Paste your OpenRouter key below. It won't appear as you type."); + Components.HintLine("You can create one at https://openrouter.ai/keys"); + AnsiConsole.WriteLine(); + return AnsiConsole.Prompt( - new TextPrompt("API key:") + new TextPrompt("Key: ") .Secret() .Validate(k => string.IsNullOrWhiteSpace(k) - ? ValidationResult.Error("[red]empty[/]") + ? ValidationResult.Error($"[{Theme.Danger}]Paste a key to continue, or press Ctrl+C to go back.[/]") : ValidationResult.Success())); } @@ -350,32 +486,46 @@ private async Task ValidateAndSaveAsync(string key, CancellationToken ct) { IReadOnlyList? models = null; await AnsiConsole.Status() - .Spinner(Spinner.Known.Dots) - .StartAsync("validating key against OpenRouter…", async _ => + .Spinner(Glyphs.Spinner) + .SpinnerStyle(new Style(Color.Grey)) + .StartAsync($"[{Theme.Muted}]Checking your key[/]", async _ => { models = await tmp.ListModelsAsync(ct); }); if (models is null || models.Count == 0) { - AnsiConsole.MarkupLine("[red]✗ empty model list[/]"); + Components.StatusCard( + Severity.Warn, + "No models came back", + "The key worked, but OpenRouter returned an empty model list.", + "This is usually temporary. Try again in a moment."); return false; } _keyStore.Save(key); - AnsiConsole.MarkupLine("[green]✓ key validated and saved.[/]"); + Components.SuccessLine("Key saved. You're ready to chat."); AnsiConsole.WriteLine(); return true; } catch (ChatException ex) when (ex.Kind == ChatErrorKind.AuthFailure) { - AnsiConsole.MarkupLine($"[red]✗ invalid key:[/] {Markup.Escape(ex.Message)}"); + Components.StatusCard( + Severity.Danger, + "That key wasn't accepted", + "OpenRouter rejected it. It may be mistyped, revoked, or from a different service.", + "Check the key at https://openrouter.ai/keys and try again."); return false; } catch (ChatException ex) when (ex.Kind == ChatErrorKind.NetworkDown) { - AnsiConsole.MarkupLine($"[red]✗ network unreachable:[/] {Markup.Escape(ex.Message)}"); - if (AnsiConsole.Confirm("save key anyway and try later?", defaultValue: false)) + Components.StatusCard( + Severity.Warn, + "Can't reach OpenRouter", + ex.Message, + "OpenKey can save the key now and check it the first time you chat."); + + if (AnsiConsole.Confirm("Save the key and continue?", defaultValue: false)) { _keyStore.Save(key); return true; @@ -384,7 +534,11 @@ await AnsiConsole.Status() } catch (ChatException ex) { - AnsiConsole.MarkupLine($"[red]✗ validation failed ({ex.Kind}):[/] {Markup.Escape(ex.Message)}"); + Components.StatusCard( + Severity.Warn, + "Couldn't check the key", + ex.Message, + "Try again, or paste a different key."); return false; } } @@ -403,10 +557,14 @@ private async Task ResetAllAsync(CancellationToken ct) } catch (Exception ex) when (ex is IOException or UnauthorizedAccessException) { - AnsiConsole.MarkupLine($"[yellow]warn: could not fully remove {Markup.Escape(_paths.RootDir)}: {Markup.Escape(ex.Message)}[/]"); + Components.StatusCard( + Severity.Warn, + "Some files couldn't be removed", + $"OpenKey cleared what it could from {_paths.RootDir}, but something is holding the rest.", + "Close any other copy of OpenKey and try again."); } - AnsiConsole.MarkupLine("[green]✓ reset complete.[/]"); + Components.SuccessLine("Everything was cleared."); AnsiConsole.WriteLine(); if (await EnsureFirstRunAsync(ct)) @@ -414,9 +572,132 @@ private async Task ResetAllAsync(CancellationToken ct) try { await _catalog.RefreshAsync(ct); } catch (ChatException) { /* will retry on first turn */ } ClearAndShowChatHeader(); await _engine.ResumeAsync(ct); + return; } + + // Setup was abandoned, so there is no key. Returning to the REPL here would strand the user + // in a loop: every message fails auth, the error says "run /reset", and /reset lands back + // exactly here. Exiting is the only honest option. + AnsiConsole.WriteLine(); + AnsiConsole.MarkupLine("OpenKey needs a key to work, so it will close now."); + Components.HintLine("Start it again when you're ready to sign in."); + _exiting = true; + } + + /// + /// Reads one chat line. Uses rather than a Spectre prompt for + /// three reasons: it gets the Windows console's native line editor (arrows, Home/End, word + /// jump, F7 history) which Spectre's reader does not implement; it does not throw when output + /// is redirected, which Spectre prompts now do; and a multi-line paste leaves its remaining + /// lines in the driver buffer where they can be drained instead of being executed as commands. + /// + private static string? ReadUserLine() + { + AnsiConsole.WriteLine(); + AnsiConsole.Markup(UserPrompt); + + var first = Console.ReadLine(); + + // A real console echoes the typed line and its newline; a redirected stdin does not, so + // without this the next output continues on the prompt row. + if (Console.IsInputRedirected) AnsiConsole.WriteLine(); + + if (first is null) return null; + + // A human cannot type the next line within milliseconds, so input already buffered here + // means a paste. Drain it so the rest of the paste joins this message rather than being + // submitted as separate turns — one of which could start with '/' and run as a command. + if (!TryPeekBufferedInput()) return first; + + var sb = new StringBuilder(first); + while (TryPeekBufferedInput()) + { + var next = Console.ReadLine(); + if (next is null) break; + sb.Append('\n').Append(next); + } + return sb.ToString(); } + private static bool TryPeekBufferedInput() + { + try + { + for (var i = 0; i < 3; i++) + { + if (Console.KeyAvailable) return true; + Thread.Sleep(5); + } + return false; + } + catch (InvalidOperationException) + { + return false; // stdin redirected — no key buffer to inspect + } + } + + /// + /// Renders an unhandled exception and holds the window. Called from the top-level handler. + /// + public static void ReportFatal(Exception ex) + { + try + { + AnsiConsole.WriteLine(); + AnsiConsole.MarkupLine("[red]OpenKey hit an unexpected problem and has to close.[/]"); + AnsiConsole.MarkupLine($"[grey]{Markup.Escape(ex.GetType().Name)}: {Markup.Escape(ex.Message)}[/]"); + AnsiConsole.WriteLine(); + AnsiConsole.MarkupLine("[grey]If this keeps happening, run[/] /reset [grey]on the next start.[/]"); + } + catch + { + Console.WriteLine("OpenKey hit an unexpected problem and has to close."); + Console.WriteLine(ex); + } + HoldIfOwnConsole(); + } + + /// + /// When OpenKey owns its console — i.e. it was double-clicked rather than run from an existing + /// terminal — the window dies with the process, so any parting message is unreadable by + /// construction. Hold it open in that case only; never when run from a shell or a script. + /// + private static void HoldIfOwnConsole() + { + if (!OwnsConsole()) return; + try + { + AnsiConsole.WriteLine(); + AnsiConsole.MarkupLine("[grey]Press any key to close.[/]"); + Console.ReadKey(intercept: true); + } + catch (InvalidOperationException) + { + // No console to wait on. + } + } + + private static bool OwnsConsole() + { + try + { + if (Console.IsOutputRedirected || Console.IsInputRedirected) return false; + var buffer = new uint[4]; + var count = GetConsoleProcessList(buffer, (uint)buffer.Length); + return count == 1; // only us attached => we created this window + } + catch (Exception ex) when (ex is DllNotFoundException or EntryPointNotFoundException or InvalidOperationException) + { + return false; + } + } + + // DllImport rather than LibraryImport: the latter requires AllowUnsafeBlocks project-wide, + // which is a lot of permission to buy for one blittable call. + [System.Runtime.InteropServices.DllImport("kernel32.dll", SetLastError = true)] + private static extern uint GetConsoleProcessList( + [System.Runtime.InteropServices.Out] uint[] processList, uint processCount); + private enum OAuthOutcomeKind { KeyAcquired, diff --git a/src/OpenKey/DpapiKeyStore.cs b/src/OpenKey/DpapiKeyStore.cs index 8d95d46..2755df8 100644 --- a/src/OpenKey/DpapiKeyStore.cs +++ b/src/OpenKey/DpapiKeyStore.cs @@ -1,6 +1,7 @@ using System.Security.Cryptography; using System.Text; using OpenKey.Core.AppPaths; +using OpenKey.Core.Providers; using OpenKey.Core.Storage; namespace OpenKey; @@ -32,17 +33,47 @@ public sealed class DpapiKeyStore : IKeyStore } } + /// + /// Unlike the other stores, a failure here is NOT swallowed. Session history and rotation state + /// are conveniences, but a key that silently fails to persist means signing in again on every + /// launch with no explanation — the user has to be told. + /// public void Save(string apiKey) { - _paths.EnsureRoot(); - var plain = Encoding.UTF8.GetBytes(apiKey); - var cipher = ProtectedData.Protect(plain, Entropy, DataProtectionScope.CurrentUser); - File.WriteAllBytes(_paths.KeyFile, cipher); + try + { + _paths.EnsureRoot(); + var plain = Encoding.UTF8.GetBytes(apiKey); + var cipher = ProtectedData.Protect(plain, Entropy, DataProtectionScope.CurrentUser); + File.WriteAllBytes(_paths.KeyFile, cipher); + } + catch (Exception ex) when (ex is IOException or UnauthorizedAccessException) + { + throw new ChatException( + ChatErrorKind.MalformedResponse, + $"Couldn't save your key to {_paths.RootDir}. Check the folder is writable and the disk isn't full.", + null, + ex); + } + catch (CryptographicException ex) + { + throw new ChatException( + ChatErrorKind.MalformedResponse, + "Windows wouldn't encrypt the key for this account.", + null, + ex); + } } public void Clear() { - var path = _paths.KeyFile; - if (File.Exists(path)) File.Delete(path); + try + { + var path = _paths.KeyFile; + if (File.Exists(path)) File.Delete(path); + } + catch (Exception ex) when (ex is IOException or UnauthorizedAccessException) + { + } } } diff --git a/src/OpenKey/MarkdownConsoleRenderer.cs b/src/OpenKey/MarkdownConsoleRenderer.cs index fb47a11..5d6f21f 100644 --- a/src/OpenKey/MarkdownConsoleRenderer.cs +++ b/src/OpenKey/MarkdownConsoleRenderer.cs @@ -2,14 +2,14 @@ using Markdig; using Markdig.Syntax; using Markdig.Syntax.Inlines; +using OpenKey.Ui; using Spectre.Console; namespace OpenKey; /// -/// Renders an LLM markdown reply to the console using Spectre. Parses once (Markdig) -/// after the full reply is buffered — no per-token re-parse. All literal text is escaped -/// so model output can never inject Spectre markup. +/// Renders a markdown block to the console using Spectre. Every literal is escaped, so model +/// output can never inject Spectre markup — the fallback path re-escapes too. /// internal static class MarkdownConsoleRenderer { @@ -18,7 +18,7 @@ public static void Render(IAnsiConsole console, string markdown) try { var doc = Markdown.Parse(markdown); - RenderBlocks(console, doc, indent: 0); + RenderBlocks(console, doc, indent: 0, topLevel: true); } catch (Exception) { @@ -28,10 +28,17 @@ public static void Render(IAnsiConsole console, string markdown) } } - private static void RenderBlocks(IAnsiConsole console, ContainerBlock container, int indent) + private static void RenderBlocks(IAnsiConsole console, ContainerBlock container, int indent, bool topLevel) { + var first = true; foreach (var block in container) + { + // One blank line between top-level blocks, never zero and never two. Previously the + // renderer emitted none at all, so headings, paragraphs and lists butted together. + if (topLevel && !first) console.WriteLine(); RenderBlock(console, block, indent); + first = false; + } } private static void RenderBlock(IAnsiConsole console, Block block, int indent) @@ -41,8 +48,10 @@ private static void RenderBlock(IAnsiConsole console, Block block, int indent) { case HeadingBlock h: { - var color = h.Level <= 2 ? "cyan" : "white"; - console.MarkupLine($"{pad}[bold {color}]{InlineToMarkup(h.Inline)}[/]"); + // H1-H2 are structural, so they take the brand colour; deeper headings are just + // emphasis and stay uncoloured. "white" would break a light-background console. + var style = h.Level <= 2 ? Theme.BrandStrong : Theme.Strong; + console.MarkupLine($"{pad}[{style}]{InlineToMarkup(h.Inline)}[/]"); break; } case FencedCodeBlock fenced: @@ -55,7 +64,7 @@ private static void RenderBlock(IAnsiConsole console, Block block, int indent) foreach (var child in quote) { if (child is LeafBlock lb && lb.Inline is not null) - console.MarkupLine($"{pad}[grey]│ {InlineToMarkup(lb.Inline)}[/]"); + console.MarkupLine($"{pad}[{Theme.Muted}]{Glyphs.QuoteBar} {InlineToMarkup(lb.Inline)}[/]"); else RenderBlock(console, child, indent); } @@ -64,13 +73,13 @@ private static void RenderBlock(IAnsiConsole console, Block block, int indent) RenderList(console, list, indent); break; case ThematicBreakBlock: - console.Write(new Rule().RuleStyle("grey")); + console.Write(new Rule().RuleStyle(Theme.Muted)); break; case ParagraphBlock p: console.MarkupLine($"{pad}{InlineToMarkup(p.Inline)}"); break; case ContainerBlock cont: - RenderBlocks(console, cont, indent); + RenderBlocks(console, cont, indent, topLevel: false); break; case LeafBlock leaf when leaf.Inline is not null: console.MarkupLine($"{pad}{InlineToMarkup(leaf.Inline)}"); @@ -84,19 +93,21 @@ private static void RenderList(IAnsiConsole console, ListBlock list, int indent) foreach (var item in list) { if (item is not ListItemBlock li) continue; - var bullet = list.IsOrdered ? $"{n}." : "•"; + var bullet = list.IsOrdered ? $"{n}." : Glyphs.Bullet; n++; - var first = true; + var firstChild = true; foreach (var child in li) { - if (first && child is ParagraphBlock p) + if (firstChild && child is ParagraphBlock p) { - console.MarkupLine($"{new string(' ', indent)}[grey]{bullet}[/] {InlineToMarkup(p.Inline)}"); - first = false; + console.MarkupLine( + $"{new string(' ', indent)}[{Theme.Muted}]{bullet}[/] {InlineToMarkup(p.Inline)}"); + firstChild = false; } else { + // Nested content hangs under the item text: bullet + space = 2 columns. RenderBlock(console, child, indent + 2); } } @@ -107,11 +118,11 @@ private static void RenderCode(IAnsiConsole console, string code, string? lang) { var body = code.TrimEnd('\n', '\r'); var panel = new Panel(new Text(body)) - .Border(BoxBorder.Rounded) + .Border(Glyphs.Box) .BorderColor(Color.Grey) .Expand(); if (!string.IsNullOrWhiteSpace(lang)) - panel.Header($" {Markup.Escape(lang)} "); + panel.Header($"[{Theme.Muted}] {Markup.Escape(lang.Trim().ToLowerInvariant())} [/]"); console.Write(panel); } @@ -141,22 +152,30 @@ private static void AppendInline(StringBuilder sb, Inline inline) break; } case CodeInline code: - sb.Append("[white on grey23]").Append(Markup.Escape(code.Content)).Append("[/]"); + // Foreground only. The old "white on grey23" downsampled to white-on-black in + // 16-colour mode: invisible on light schemes, identical to body text on dark ones, + // so the one style meant to make code stand out did nothing. + sb.Append('[').Append(Theme.Code).Append(']') + .Append(Markup.Escape(code.Content)).Append("[/]"); break; case LinkInline link: { var label = new StringBuilder(); foreach (var child in link) AppendInline(label, child); + // Escape the URL rather than dropping links whose URL contains a bracket. The old + // filter silently discarded the target of any such link — legal in a URL, and + // common in generated ones. var url = link.Url ?? string.Empty; - if (!string.IsNullOrEmpty(url) && !url.Contains('[') && !url.Contains(']')) - sb.Append("[link=").Append(url).Append(']').Append(label).Append("[/]"); + if (!string.IsNullOrEmpty(url)) + sb.Append("[link=").Append(Markup.Escape(url)).Append(']').Append(label).Append("[/]"); else sb.Append(label); break; } case AutolinkInline auto: - sb.Append("[link]").Append(Markup.Escape(auto.Url)).Append("[/]"); + sb.Append("[link=").Append(Markup.Escape(auto.Url)).Append(']') + .Append(Markup.Escape(auto.Url)).Append("[/]"); break; case LineBreakInline: sb.Append('\n'); diff --git a/src/OpenKey/OAuth/OAuthPortInUseException.cs b/src/OpenKey/OAuth/OAuthPortInUseException.cs index acc08c4..d7cd024 100644 --- a/src/OpenKey/OAuth/OAuthPortInUseException.cs +++ b/src/OpenKey/OAuth/OAuthPortInUseException.cs @@ -5,7 +5,7 @@ public sealed class OAuthPortInUseException : Exception public int Port { get; } public OAuthPortInUseException(int port, Exception inner) - : base($"Port {port} is in use by another app on this machine, so OpenKey can't receive the OAuth callback. Close the other app or use the paste flow.", inner) + : base("Every port OpenKey can use for browser sign-in is already taken by another app on this machine.", inner) { Port = port; } diff --git a/src/OpenKey/OAuth/OAuthWire.cs b/src/OpenKey/OAuth/OAuthWire.cs new file mode 100644 index 0000000..c051118 --- /dev/null +++ b/src/OpenKey/OAuth/OAuthWire.cs @@ -0,0 +1,15 @@ +using System.Text.Json.Serialization; + +namespace OpenKey.OAuth; + +/// +/// Body for POST /api/v1/auth/keys, the PKCE code-for-key exchange documented in +/// docs/03-openrouter-integration.md. Named rather than anonymous so it can be source-generated. +/// +internal sealed record KeyExchangeRequest( + [property: JsonPropertyName("code")] string Code, + [property: JsonPropertyName("code_verifier")] string CodeVerifier, + [property: JsonPropertyName("code_challenge_method")] string CodeChallengeMethod); + +[JsonSerializable(typeof(KeyExchangeRequest))] +internal sealed partial class OAuthJsonContext : JsonSerializerContext; diff --git a/src/OpenKey/OAuth/OpenRouterOAuth.cs b/src/OpenKey/OAuth/OpenRouterOAuth.cs index 254007f..6e4b96a 100644 --- a/src/OpenKey/OAuth/OpenRouterOAuth.cs +++ b/src/OpenKey/OAuth/OpenRouterOAuth.cs @@ -12,11 +12,23 @@ public sealed class OpenRouterOAuth { private const string AuthBase = "https://openrouter.ai/auth"; private const string ExchangeUrl = "https://openrouter.ai/api/v1/auth/keys"; - private const int CallbackPort = 3000; - private const string CallbackUrl = "http://localhost:3000/callback"; - private const string ListenerPrefix = "http://localhost:3000/callback/"; + /// + /// Callback ports to try, in order. + /// + /// The port cannot simply be randomised: OpenRouter upserts an app record keyed by callback + /// URL, so a varying callback returns 409. But a small fixed set is fine — each entry + /// is a stable URL that can be registered once. Previously port 3000 was the only option, and + /// anything else holding it (a dev server, very commonly) left pasting a key as the only route. + /// + /// + private static readonly int[] CallbackPorts = { 3000, 3123, 8321, 8765 }; + private static readonly TimeSpan CallbackTimeout = TimeSpan.FromMinutes(5); + private static string CallbackUrlFor(int port) => $"http://localhost:{port}/callback"; + + private static string ListenerPrefixFor(int port) => $"http://localhost:{port}/callback/"; + private readonly HttpClient _http; public OpenRouterOAuth(HttpClient http) => _http = http; @@ -25,21 +37,37 @@ public async Task AcquireKeyAsync(Action onAuthUrl, Cancellation { var pkce = PkceCodes.Generate(); - // Fixed callback URL per OpenRouter docs (recommended for local-first apps). - // Varying the callback per attempt causes "Failed to create or update app" 409s server-side. - using var listener = new HttpListener(); - listener.Prefixes.Add(ListenerPrefix); - try - { - listener.Start(); - } - catch (HttpListenerException ex) + // Try each known callback port until one binds. Each is a fixed URL, so OpenRouter's + // app-record upsert still sees a stable callback and does not 409. + HttpListener? bound = null; + var boundPort = 0; + HttpListenerException? lastBindFailure = null; + + foreach (var port in CallbackPorts) { - throw new OAuthPortInUseException(CallbackPort, ex); + var candidate = new HttpListener(); + candidate.Prefixes.Add(ListenerPrefixFor(port)); + try + { + candidate.Start(); + bound = candidate; + boundPort = port; + break; + } + catch (HttpListenerException ex) + { + lastBindFailure = ex; + candidate.Close(); + } } + if (bound is null) + throw new OAuthPortInUseException(CallbackPorts[0], lastBindFailure!); + + using var listener = bound; + var authUrl = - $"{AuthBase}?callback_url={Uri.EscapeDataString(CallbackUrl)}" + + $"{AuthBase}?callback_url={Uri.EscapeDataString(CallbackUrlFor(boundPort))}" + $"&code_challenge={Uri.EscapeDataString(pkce.CodeChallenge)}" + $"&code_challenge_method={PkceCodes.ChallengeMethod}"; @@ -151,13 +179,8 @@ private async Task ExchangeCodeAsync(string code, string verifier, Cance private Task PostExchangeAsync(string code, string verifier, CancellationToken ct) { - var body = new - { - code, - code_verifier = verifier, - code_challenge_method = PkceCodes.ChallengeMethod, - }; - return _http.PostAsJsonAsync(ExchangeUrl, body, ct); + var body = new KeyExchangeRequest(code, verifier, PkceCodes.ChallengeMethod); + return _http.PostAsJsonAsync(ExchangeUrl, body, OAuthJsonContext.Default.KeyExchangeRequest, ct); } private static string ParseKey(string text) diff --git a/src/OpenKey/OpenKey.csproj b/src/OpenKey/OpenKey.csproj index e3aa7e6..a6c45c2 100644 --- a/src/OpenKey/OpenKey.csproj +++ b/src/OpenKey/OpenKey.csproj @@ -4,16 +4,38 @@ net10.0-windows OpenKey OpenKey - win-x64 + win-x64;win-arm64 false + + + true + true + true + true + true + + + true - - - - + + + + + + + + + + + + + diff --git a/src/OpenKey/Program.cs b/src/OpenKey/Program.cs index fbce2f8..4c59ca3 100644 --- a/src/OpenKey/Program.cs +++ b/src/OpenKey/Program.cs @@ -18,7 +18,10 @@ services.AddSingleton(); services.AddSingleton(); -services.AddSingleton(_ => new HttpClient { Timeout = TimeSpan.FromSeconds(60) }); +// Infinite on purpose. HttpClient.Timeout bounds the *entire* response including reading the body, +// even with ResponseHeadersRead, so any finite value here silently aborts long-but-healthy streamed +// replies and gets misread as a network fault. The provider applies per-read deadlines instead. +services.AddSingleton(_ => new HttpClient { Timeout = Timeout.InfiniteTimeSpan }); services.AddSingleton(sp => { @@ -28,8 +31,23 @@ }); services.AddSingleton(); +services.AddSingleton(); +services.AddSingleton(); services.AddSingleton(); services.AddSingleton(); await using var sp = services.BuildServiceProvider(); -await sp.GetRequiredService().RunAsync(); + +try +{ + await sp.GetRequiredService().RunAsync(); +} +catch (Exception ex) +{ + // Without this the window closes on the same frame as the stack trace, so a double-clicked + // OpenKey.exe just vanishes and the user has nothing to report. + ConsoleHost.ReportFatal(ex); + return 1; +} + +return 0; diff --git a/src/OpenKey/TiktokenCounter.cs b/src/OpenKey/TiktokenCounter.cs new file mode 100644 index 0000000..2ce2e48 --- /dev/null +++ b/src/OpenKey/TiktokenCounter.cs @@ -0,0 +1,53 @@ +using Microsoft.ML.Tokenizers; +using OpenKey.Core.Engine; + +namespace OpenKey; + +/// +/// Real token counting, backed by the cl100k_base vocabulary. +/// +/// The vocabulary is embedded via Microsoft.ML.Tokenizers.Data.Cl100kBase rather than +/// downloaded on demand. OpenKey must work on first run behind a captive portal and makes no +/// network call other than to OpenRouter, so a tokenizer that fetches its own vocabulary would +/// break both properties. +/// +/// +/// cl100k is a GPT-family encoding, and OpenRouter's free tier is mostly Llama, Qwen, DeepSeek and +/// Mistral, whose tokenizers differ. It is still substantially closer than four-characters-per-token +/// for ordinary prose, and this only drives how much history to drop — so being close and cheap +/// beats being exact and slow. +/// +/// +internal sealed class TiktokenCounter : ITokenCounter +{ + private readonly TiktokenTokenizer? _tokenizer; + private readonly HeuristicTokenCounter _fallback = new(); + + public TiktokenCounter() + { + try + { + _tokenizer = TiktokenTokenizer.CreateForEncoding("cl100k_base"); + } + catch (Exception) + { + // Never let tokenizer setup stop the app starting; the heuristic is a fine substitute. + _tokenizer = null; + } + } + + public int Count(string text) + { + if (string.IsNullOrEmpty(text)) return 0; + if (_tokenizer is null) return _fallback.Count(text); + + try + { + return _tokenizer.CountTokens(text); + } + catch (Exception) + { + return _fallback.Count(text); + } + } +} diff --git a/src/OpenKey/Ui/Components.cs b/src/OpenKey/Ui/Components.cs new file mode 100644 index 0000000..6eb51ee --- /dev/null +++ b/src/OpenKey/Ui/Components.cs @@ -0,0 +1,165 @@ +using System.Reflection; +using Spectre.Console; + +namespace OpenKey.Ui; + +internal enum Severity +{ + /// Failed and will not recover on its own. + Danger, + + /// Recoverable, self-healing, or user-initiated destruction. + Warn, +} + +/// +/// Every visible element in the app. Nothing is styled at a call site — that is what kept the old +/// console incoherent, with three different cases and four border styles inside single methods. +/// +internal static class Components +{ + /// + /// Display version. InformationalVersion carries a "+<commit sha>" suffix that the SDK appends; + /// it is noise on a banner a non-technical user reads, so it is trimmed off. + /// + public static string Version { get; } = BuildVersion(); + + private static string BuildVersion() + { + var raw = typeof(Components).Assembly + .GetCustomAttribute()?.InformationalVersion + ?? typeof(Components).Assembly.GetName().Version?.ToString() + ?? "dev"; + + var plus = raw.IndexOf('+', StringComparison.Ordinal); + return plus > 0 ? raw[..plus] : raw; + } + + public static void Banner() + { + AnsiConsole.Write( + new Rule($"[{Theme.BrandStrong}]OpenKey[/] [{Theme.Muted}]v{Markup.Escape(Version)}[/]") + .LeftJustified() + .RuleStyle(Theme.Muted)); + AnsiConsole.MarkupLine($"[{Theme.Muted}]Developed by Paolo Patron[/]"); + AnsiConsole.WriteLine(); + } + + /// The only thing that clears the screen. + public static void HomeHeader() + { + if (ConsoleLayout.Rich) + { + try { AnsiConsole.Clear(); } + catch (IOException) { /* handle isn't a console */ } + } + + Banner(); + HintPanel(); + AnsiConsole.WriteLine(); + } + + private static void HintPanel() + { + var body = new Markup( + "Type a message and press Enter to chat.\n" + + "\n" + + $"[{Theme.Brand}]/models[/] Choose which AI model answers you\n" + + $"[{Theme.Brand}]/new[/] Start a fresh conversation\n" + + $"[{Theme.Brand}]/help[/] See everything OpenKey can do\n" + + $"[{Theme.Brand}]/quit[/] Close OpenKey"); + + AnsiConsole.Write(new Panel(body) + .Header($"[{Theme.BrandStrong}] Getting started [/]") + .Border(Glyphs.Box) + .BorderColor(Color.Grey) + .Expand() + .Padding(1, 1, 1, 1)); + } + + /// + /// Identity line for a reply. Printed the moment the request goes out — the model is known + /// before the first byte, so there is no reason to make the user stare at nothing. + /// + public static void ReplyHeader(string modelId, TimeSpan elapsed) + { + AnsiConsole.MarkupLine( + $"[{Theme.BrandStrong}]OpenKey AI[/][{Theme.Muted}] {Glyphs.Sep} {Markup.Escape(modelId)} {Glyphs.Sep} {FormatElapsed(elapsed)}[/]"); + AnsiConsole.WriteLine(); + } + + public static string FormatElapsed(TimeSpan t) => + t.TotalSeconds < 60 + ? $"{t.TotalSeconds:0.0}s" + : $"{(int)t.TotalMinutes}m {t.Seconds}s"; + + /// + /// One quiet line noting that rotation happened. Rotation working correctly is not a warning, + /// so this is grey rather than yellow, mentions no model id (the header already carries it), + /// and never leaks a ChatErrorKind name. + /// + public static void RotationNote(int skipped) + { + if (skipped <= 0) return; + var word = skipped == 1 ? "model" : "models"; + AnsiConsole.MarkupLine($"[{Theme.Muted}]Moved past {skipped} busy {word}.[/]"); + AnsiConsole.WriteLine(); + } + + /// + /// The single error surface. Body is always two paragraphs: what happened, then what to do. + /// A card with no next step is a bug — it leaves the user at a dead end. + /// + public static void StatusCard(Severity severity, string title, string detail, string nextStep) + { + var (border, titleStyle) = severity switch + { + Severity.Warn => (Color.Yellow, $"bold {Theme.Warn}"), + _ => (Color.Red, $"bold {Theme.Danger}"), + }; + + var body = new Markup( + $"[{Theme.Muted}]{Markup.Escape(detail)}[/]\n\n{nextStep}"); + + AnsiConsole.WriteLine(); + AnsiConsole.Write(new Panel(body) + .Header($"[{titleStyle}] {Markup.Escape(title)} [/]") + .Border(Glyphs.Box) + .BorderColor(border) + .Expand() + .Padding(1, 1, 1, 1)); + AnsiConsole.WriteLine(); + } + + /// General-purpose "here is what to do" line. + public static void HintLine(string markup) => + AnsiConsole.MarkupLine($"[{Theme.Muted}]{markup}[/]"); + + /// The glyph carries the signal; the sentence stays readable at default foreground. + public static void SuccessLine(string sentence) => + AnsiConsole.MarkupLine($"[{Theme.Ok}]{Glyphs.Ok}[/] {Markup.Escape(sentence)}"); + + public static void KeyValuePanel(string header, IReadOnlyList<(string Label, string Value)> rows) + { + var grid = new Grid() + .AddColumn(new GridColumn().NoWrap().PadRight(3).Width(16)) + .AddColumn(); + + foreach (var (label, value) in rows) + grid.AddRow($"[{Theme.Muted}]{Markup.Escape(label)}[/]", Markup.Escape(value)); + + AnsiConsole.Write(new Panel(grid) + .Header($"[{Theme.BrandStrong}] {Markup.Escape(header)} [/]") + .Border(Glyphs.Box) + .BorderColor(Color.Grey) + .Expand() + .Padding(1, 1, 1, 1)); + } + + /// + /// Picker title carries its own hint: Spectre owns every row below the title, so a hint printed + /// afterwards is impossible. + /// + public static string PickerTitle(string question) => + $"{question}[{Theme.Muted}] (Up/Down to move, Enter to choose)[/]"; +} diff --git a/src/OpenKey/Ui/ConsoleLayout.cs b/src/OpenKey/Ui/ConsoleLayout.cs new file mode 100644 index 0000000..2272a48 --- /dev/null +++ b/src/OpenKey/Ui/ConsoleLayout.cs @@ -0,0 +1,90 @@ +using System.Text; +using Spectre.Console; + +namespace OpenKey.Ui; + +/// +/// Startup-time console setup. Must run before anything prints: Spectre computes and caches +/// terminal capabilities on first access, so encoding has to be settled first. +/// +internal static class ConsoleLayout +{ + /// + /// Widest line the app will draw. Capping matters more than it sounds: without it a maximized + /// 200-column terminal turns a three-line code snippet into a 200-wide box, and a reply reads + /// differently on every machine. 100 is a comfortable measure for prose. + /// + public const int MaxWidth = 100; + + /// Below this, Spectre's own wrapping takes over; no narrow layout is attempted. + public const int MinWidth = 60; + + public static int Width { get; private set; } = MaxWidth; + + /// + /// True when the console can be drawn on richly — ANSI available and not redirected. + /// + /// Both halves matter. Spectre 0.55 disables ANSI whenever stdout is redirected, and makes + /// Capabilities.Interactive false if any std stream is redirected — at which + /// point its prompts throw. Anything that moves the cursor, clears the screen, or prompts must + /// check this first. + /// + /// + public static bool Rich { get; private set; } = true; + + public static void Initialize() + { + // LLM replies are full of em-dashes, smart quotes and emoji; without UTF-8 they arrive as + // '?'. Guarded because stdout may be redirected or absent. + try { Console.OutputEncoding = Encoding.UTF8; } catch (IOException) { } catch (System.Security.SecurityException) { } + + Glyphs.Resolve(); + + var caps = AnsiConsole.Profile.Capabilities; + Rich = caps.Ansi && caps.Interactive; + + Width = MaxWidth; + try + { + if (!Console.IsOutputRedirected) + Width = Math.Clamp(Console.WindowWidth, MinWidth, MaxWidth); + } + catch (IOException) + { + // No console attached; keep the default. + } + + AnsiConsole.Profile.Width = Width; + } + + /// + /// Re-reads the terminal width. Spectre reads Console.WindowWidth live on every access, + /// so a resize changes new output but never reflows what is already drawn. + /// + public static int CurrentWidth() + { + try + { + if (Console.IsOutputRedirected) return MaxWidth; + return Math.Clamp(Console.WindowWidth, MinWidth, MaxWidth); + } + catch (IOException) + { + return Width; + } + } + + /// Viewport height, used to decide whether a block can still be erased. + public static int CurrentHeight() + { + try + { + if (Console.IsOutputRedirected) return int.MaxValue; + return Math.Max(1, Console.WindowHeight); + } + catch (IOException) + { + return int.MaxValue; + } + } +} diff --git a/src/OpenKey/Ui/Glyphs.cs b/src/OpenKey/Ui/Glyphs.cs new file mode 100644 index 0000000..211e1dd --- /dev/null +++ b/src/OpenKey/Ui/Glyphs.cs @@ -0,0 +1,61 @@ +using Spectre.Console; + +namespace OpenKey.Ui; + +/// +/// Non-ASCII characters the app draws, resolved once at startup into a tier. +/// +/// Windows Terminal has DirectWrite font fallback and renders anything. The GDI-rendered legacy +/// conhost does not fall back for glyphs missing from Consolas — it draws a box instead. So the +/// tier check is deliberately conservative: full Unicode only when we can see we are in Windows +/// Terminal with a UTF-8 codepage. +/// +/// A glyph literal anywhere outside this type is a defect. +/// +internal static class Glyphs +{ + /// + /// True only for Windows Terminal on codepage 65001. Conservative on purpose — the cost of a + /// false positive is a row of boxes on every prompt. + /// + public static bool Unicode { get; private set; } + + /// Prompt caret. The single highest-risk glyph in the app: U+276F is in neither CP437 nor Consolas. + public static string Caret { get; private set; } = ">"; + + public static string Ok { get; private set; } = "+"; + public static string Fail { get; private set; } = "x"; + + /// Separator. Safe in both tiers — CP437 0xFA, CP1252 0xB7. + public static string Sep => "·"; + + /// List bullet. A hyphen in both tiers: calmer than U+2022 and safe outside CP437. + public static string Bullet => "-"; + + /// Blockquote bar. Safe in both tiers — CP437. + public static string QuoteBar => "│"; + + public static string Ellipsis => "…"; + + /// + /// Braille dots on the Unicode tier only. Spinner.Known.Dots is U+28xx and absent from + /// Consolas, and it runs on every single turn — the most likely visible breakage in the app. + /// SimpleDots is pure ASCII and calmer than Line, which spins fast and reads retro-toy. + /// + public static Spinner Spinner => Unicode ? Spectre.Console.Spinner.Known.Dots : Spectre.Console.Spinner.Known.SimpleDots; + + /// Square borders in both tiers: the rounded set (U+256D–2570) is outside CP437. + public static BoxBorder Box => BoxBorder.Square; + + public static TableBorder Table => TableBorder.Square; + + public static void Resolve() + { + Unicode = Console.OutputEncoding.CodePage == 65001 + && Environment.GetEnvironmentVariable("WT_SESSION") is not null; + + Caret = Unicode ? "❯" : ">"; + Ok = Unicode ? "✓" : "+"; + Fail = Unicode ? "✗" : "x"; + } +} diff --git a/src/OpenKey/Ui/TextWidth.cs b/src/OpenKey/Ui/TextWidth.cs new file mode 100644 index 0000000..2e5ed27 --- /dev/null +++ b/src/OpenKey/Ui/TextWidth.cs @@ -0,0 +1,72 @@ +using System.Globalization; +using System.Text; + +namespace OpenKey.Ui; + +/// +/// How many terminal cells a run of text occupies. +/// +/// Needed because string.Length is wrong in three ways that all show up in LLM output: +/// CJK and emoji occupy two cells, combining marks and zero-width joiners occupy none, and a +/// surrogate pair is two chars but one glyph. Getting this wrong makes the streaming rewind erase +/// the wrong number of rows, which damages the transcript above. +/// +/// +/// Spectre has an equivalent calculator but it is internal in 0.57.2, so this is deliberately +/// self-contained. +/// +/// +internal static class TextWidth +{ + /// Cells occupied by a single rune: 0 for zero-width, 2 for wide, otherwise 1. + public static int Of(Rune rune) + { + var value = rune.Value; + + if (value == 0) return 0; + + // Combining marks, joiners and variation selectors render into the preceding cell. + var category = Rune.GetUnicodeCategory(rune); + if (category is UnicodeCategory.NonSpacingMark + or UnicodeCategory.SpacingCombiningMark + or UnicodeCategory.EnclosingMark + or UnicodeCategory.Format) + { + return 0; + } + + if (Rune.IsControl(rune)) return 0; + + return IsWide(value) ? 2 : 1; + } + + /// Cells occupied by a string, ignoring control characters. + public static int Of(string text) + { + var total = 0; + foreach (var rune in text.EnumerateRunes()) total += Of(rune); + return total; + } + + /// + /// East Asian Wide and Fullwidth ranges, plus the emoji blocks that terminals render double + /// width. Ranges rather than a full property table: this only has to be right for text a chat + /// model actually emits. + /// + private static bool IsWide(int cp) => + (cp >= 0x1100 && cp <= 0x115F) || // Hangul Jamo + (cp >= 0x2E80 && cp <= 0x303E) || // CJK radicals, Kangxi + (cp >= 0x3041 && cp <= 0x33FF) || // Hiragana, Katakana, CJK compatibility + (cp >= 0x3400 && cp <= 0x4DBF) || // CJK Extension A + (cp >= 0x4E00 && cp <= 0x9FFF) || // CJK Unified Ideographs + (cp >= 0xA000 && cp <= 0xA4CF) || // Yi + (cp >= 0xAC00 && cp <= 0xD7A3) || // Hangul syllables + (cp >= 0xF900 && cp <= 0xFAFF) || // CJK compatibility ideographs + (cp >= 0xFE10 && cp <= 0xFE19) || // vertical forms + (cp >= 0xFE30 && cp <= 0xFE6F) || // CJK compatibility forms + (cp >= 0xFF00 && cp <= 0xFF60) || // fullwidth forms + (cp >= 0xFFE0 && cp <= 0xFFE6) || + (cp >= 0x1F300 && cp <= 0x1F64F) || // emoji, emoticons + (cp >= 0x1F900 && cp <= 0x1F9FF) || // supplemental symbols and pictographs + (cp >= 0x20000 && cp <= 0x3FFFD); // CJK Extension B and beyond +} diff --git a/src/OpenKey/Ui/Theme.cs b/src/OpenKey/Ui/Theme.cs new file mode 100644 index 0000000..6108164 --- /dev/null +++ b/src/OpenKey/Ui/Theme.cs @@ -0,0 +1,113 @@ +namespace OpenKey.Ui; + +/// +/// Every colour the app uses, as Spectre style names. +/// +/// Governing rule: one accent, one neutral, three signals. Colour carries meaning, never +/// decoration; body text is never coloured; no background colours anywhere. +/// +/// +/// Only the 16 base ANSI colours appear here. Legacy conhost downsamples anything richer, and the +/// "nicer" greys (grey19, grey23) land on black or silver depending on the user's scheme — so they +/// either vanish or invert. Base-16 renders identically everywhere, which is also why no +/// capability-degradation step is needed: there is nothing left to downsample, and Spectre strips +/// SGR itself under NO_COLOR. +/// +/// +/// Accessibility. Colour is never the only signal. Roughly 8% of men cannot reliably +/// separate red from green, so every state that uses those also carries a glyph or a worded title: +/// success is ✓ …, failure is ✗ …, and error cards name the problem in their header. +/// Removing all colour must never remove information — that is what the mono palette tests. +/// +/// A colour literal anywhere outside this type is a defect. +/// +internal static class Theme +{ + /// Wordmark, AI label, caret, command tokens, model ids, panel titles. + public static string Brand { get; private set; } = "aqua"; + + /// Names something: labels, panel headers, column heads. + public static string BrandStrong { get; private set; } = "bold aqua"; + + /// Bold with no colour — markdown strong, the username. + public static string Strong => "bold"; + + /// Hints, elapsed time, borders, secondary detail. Never something the user must act on. + public static string Muted { get; private set; } = "grey"; + + /// The success glyph, and at most one leading word. Never a whole sentence. + public static string Ok { get; private set; } = "green"; + + /// Card border and title when the state is recoverable. + public static string Warn { get; private set; } = "yellow"; + + /// Card border and title when the state is fatal. Never body text. + public static string Danger { get; private set; } = "red"; + + /// Markdown inline code. Foreground only — no background. + public static string Code { get; private set; } = "aqua"; + + /// Currently applied palette name. + public static string Current { get; private set; } = OpenKeyConfigThemes.Default; + + // Body text deliberately has no entry: it must inherit the terminal's own foreground. + // Hardcoding white breaks every light-background console. + + public static bool IsKnown(string name) => OpenKeyConfigThemes.All.Contains(name, StringComparer.OrdinalIgnoreCase); + + public static void Apply(string name) + { + switch (name?.Trim().ToLowerInvariant()) + { + case OpenKeyConfigThemes.Light: + // Aqua and yellow are washed out on a white background; the darker pair holds up. + Brand = "blue"; + BrandStrong = "bold blue"; + Muted = "grey"; + Ok = "green"; + Warn = "olive"; + Danger = "maroon"; + Code = "blue"; + Current = OpenKeyConfigThemes.Light; + break; + + case OpenKeyConfigThemes.Mono: + // No hue at all. Everything must still be legible, which is the real test that + // meaning never depended on colour in the first place. + Brand = "default"; + BrandStrong = "bold"; + Muted = "grey"; + Ok = "default"; + Warn = "default"; + Danger = "bold"; + Code = "default"; + Current = OpenKeyConfigThemes.Mono; + break; + + case OpenKeyConfigThemes.Dark: + case OpenKeyConfigThemes.Default: + default: + Brand = "aqua"; + BrandStrong = "bold aqua"; + Muted = "grey"; + Ok = "green"; + Warn = "yellow"; + Danger = "red"; + Code = "aqua"; + Current = name?.Trim().ToLowerInvariant() == OpenKeyConfigThemes.Dark + ? OpenKeyConfigThemes.Dark + : OpenKeyConfigThemes.Default; + break; + } + } +} + +internal static class OpenKeyConfigThemes +{ + public const string Default = "default"; + public const string Dark = "dark"; + public const string Light = "light"; + public const string Mono = "mono"; + + public static readonly string[] All = { Default, Dark, Light, Mono }; +} diff --git a/src/OpenKey/Ui/TranscriptWriter.cs b/src/OpenKey/Ui/TranscriptWriter.cs new file mode 100644 index 0000000..2c47a07 --- /dev/null +++ b/src/OpenKey/Ui/TranscriptWriter.cs @@ -0,0 +1,286 @@ +using System.Text; +using Spectre.Console; + +namespace OpenKey.Ui; + +/// +/// Streams a reply at block granularity: raw text appears as it arrives, and each time a markdown +/// block completes the raw lines are erased and replaced with the styled rendering. +/// +/// Why not LiveDisplay. Spectre's live region clamps to the viewport and discards +/// overflow lines rather than scrolling them, and when the region shrinks it issues +/// EraseInDisplay(2) followed by ClearScrollback() — which would wipe the chat +/// transcript. It also has no fallback when output is redirected. None of that is survivable for a +/// scrolling conversation, so this writer drives the cursor directly instead. +/// +/// +/// The unstyled trailing text is intentional, not a limitation: styled means settled, raw means +/// still arriving. +/// +/// +internal sealed class TranscriptWriter +{ + private readonly IAnsiConsole _console; + private readonly StringBuilder _pending = new(); + + /// Raw rows emitted since the last flush, including wrapped continuation rows. + private int _rawRows; + + /// Column the raw cursor sits at, in terminal cells. + private int _col; + + /// Width sampled when the current block started; a resize invalidates the rewind. + private int _blockWidth; + + private bool _widthChanged; + private bool _inFence; + private bool _anyOutput; + + /// Styled blocks emitted so far. Distinct from , which also + /// counts raw streamed text that gets erased again. + private int _blocksRendered; + + /// + /// Whether this writer may stream raw text and erase it again. + /// + /// Judged solely from the console it was handed. It previously also consulted + /// ConsoleLayout.Rich, which is global mutable state no caller controls — so the same + /// code streamed raw text on one machine and not on another, and the tests could not pin it + /// down. caps.Ansi is already false whenever output is redirected, which is the only + /// thing the global added. + /// + /// + private readonly bool _canRewind; + + public TranscriptWriter(IAnsiConsole console) + { + _console = console; + _canRewind = console.Profile.Capabilities.Ansi; + _blockWidth = ConsoleLayout.CurrentWidth(); + } + + /// True once any text has been written, styled or raw. + public bool HasContent => _anyOutput || _pending.Length > 0; + + public void Append(string delta) + { + if (string.IsNullOrEmpty(delta)) return; + + _pending.Append(delta); + + // Never stream a fence raw, and note the test is "does the pending text contain a fence + // marker at all", not "are we currently inside one". A single delta can carry both the + // opening and closing marker, which nets to not-inside — and that used to let a whole + // code block reach the screen as raw backticks before the flush replaced it. + UpdateFenceState(delta); + if (!_inFence && !HasFenceMarker()) WriteRaw(delta); + + FlushCompletedBlocks(); + } + + /// Renders whatever is left, including an unterminated fence. + public void Complete() + { + var rest = _pending.ToString(); + _pending.Clear(); + if (rest.Trim().Length == 0) + { + if (_rawRows > 0) Rewind(); + return; + } + + Rewind(); + RenderBlock(rest); + _inFence = false; + } + + /// + /// Discards everything from an abandoned attempt. Called on + /// — without it a mid-reply rotation + /// renders the answer twice concatenated. + /// + public void Reset() + { + Rewind(); + _pending.Clear(); + _inFence = false; + _blocksRendered = 0; + _anyOutput = false; + } + + private void FlushCompletedBlocks() + { + while (true) + { + var text = _pending.ToString(); + var cut = FindBlockEnd(text); + if (cut < 0) return; + + var block = text[..cut]; + _pending.Remove(0, cut); + + if (block.Trim().Length == 0) continue; + + Rewind(); + RenderBlock(block); + } + } + + /// + /// Returns the index just past a complete block, or -1. Outside a fence a block ends at a blank + /// line; a fence ends only at its closing marker, so a blank line inside code never splits it. + /// + private static int FindBlockEnd(string text) + { + var fenceOpen = false; + var lineStart = 0; + + for (var i = 0; i < text.Length; i++) + { + if (text[i] != '\n') continue; + + var line = text.AsSpan(lineStart, i - lineStart).TrimEnd('\r'); + + if (line.TrimStart().StartsWith("```", StringComparison.Ordinal)) + { + if (fenceOpen) return i + 1; // closing fence completes the block + fenceOpen = true; + } + else if (!fenceOpen && line.Trim().IsEmpty && lineStart > 0) + { + return i + 1; // blank line completes a prose block + } + + lineStart = i + 1; + } + + return -1; + } + + private bool HasFenceMarker() + { + for (var i = 0; i + 2 < _pending.Length; i++) + { + if (_pending[i] == '`' && _pending[i + 1] == '`' && _pending[i + 2] == '`') return true; + } + return false; + } + + private void UpdateFenceState(string delta) + { + var text = _pending.ToString(); + var open = false; + var lineStart = 0; + for (var i = 0; i <= text.Length; i++) + { + if (i < text.Length && text[i] != '\n') continue; + var line = text.AsSpan(lineStart, Math.Min(i, text.Length) - lineStart).TrimEnd('\r'); + if (line.TrimStart().StartsWith("```", StringComparison.Ordinal)) open = !open; + lineStart = i + 1; + } + _inFence = open; + } + + private void WriteRaw(string delta) + { + if (!_canRewind) + { + // Nothing could be erased later, so emit nothing now and let the styled render at each + // block boundary be the only output. + return; + } + + var width = ConsoleLayout.CurrentWidth(); + if (width != _blockWidth) _widthChanged = true; + + _console.Write(delta); + _anyOutput = true; + CountRows(delta, width); + } + + /// + /// Tracks how many terminal rows the raw text occupies. Counting characters would be wrong: + /// CJK and emoji are double-width, combining marks are zero-width, and tabs jump to stops. + /// Wrapping is computed at width - 1 because terminals disagree about whether a glyph + /// landing exactly on the last column wraps immediately (conhost) or is deferred (Windows + /// Terminal) — staying a column short makes the count agree with both. + /// + private void CountRows(string text, int width) + { + var usable = Math.Max(1, width - 1); + + foreach (var rune in text.EnumerateRunes()) + { + if (rune.Value == '\n') { _rawRows++; _col = 0; continue; } + if (rune.Value == '\r') { _col = 0; continue; } + + if (rune.Value == '\t') + { + var next = ((_col / 8) + 1) * 8; + if (next >= usable) { _rawRows++; _col = 0; } else { _col = next; } + continue; + } + + var w = TextWidth.Of(rune); + if (w <= 0) continue; // combining mark / ZWJ + if (_col + w > usable) { _rawRows++; _col = 0; } + _col += w; + } + } + + /// + /// Erases the raw text written since the last flush so the styled version can replace it. + /// Skipped — leaving the raw text in place — whenever erasing would be wrong rather than + /// merely ugly. + /// + private void Rewind() + { + var rows = _rawRows; + var col = _col; + _rawRows = 0; + _col = 0; + + if (!_canRewind) return; + if (rows == 0 && col == 0) return; + + // A resize mid-block invalidates the row count, and an incorrect rewind erases unrelated + // transcript above. Losing the restyle is much cheaper than eating the conversation. + if (_widthChanged) + { + _widthChanged = false; + _console.WriteLine(); + return; + } + + // Past the viewport the earlier rows are already in scrollback, where no escape sequence + // can reach them. Leave the raw text and let the styled copy follow below. + if (rows > ConsoleLayout.CurrentHeight() - 2) + { + _console.WriteLine(); + return; + } + + // _canRewind already required caps.Ansi, so the escape sequence is safe here. Emitting it + // via ControlCode rather than a raw Console.Write keeps it inside the capability gate. + var caps = _console.Profile.Capabilities; + _console.Write(ControlCode.Create(caps, w => + { + w.Write("\r"); + if (rows > 0) w.CursorUp(rows); + w.EraseInDisplay(0); // CSI 0 J — cursor to end of screen. Never 2, never scrollback. + })); + } + + private void RenderBlock(string block) + { + // One blank line between blocks, never before the first — the vertical rhythm unit is a + // single blank line, never zero and never two. + if (_blocksRendered > 0) _console.WriteLine(); + + MarkdownConsoleRenderer.Render(_console, block.TrimEnd('\n', '\r')); + _blocksRendered++; + _anyOutput = true; + _blockWidth = ConsoleLayout.CurrentWidth(); + _widthChanged = false; + } +} diff --git a/tests/OpenKey.Core.Tests/ChatEngineTests.cs b/tests/OpenKey.Core.Tests/ChatEngineTests.cs new file mode 100644 index 0000000..c8dd7d7 --- /dev/null +++ b/tests/OpenKey.Core.Tests/ChatEngineTests.cs @@ -0,0 +1,217 @@ +using OpenKey.Core.AppPaths; +using OpenKey.Core.Engine; +using OpenKey.Core.Providers; +using OpenKey.Core.Storage; +using Xunit; + +namespace OpenKey.Core.Tests; + +/// +/// Covers the retry/rotation state machine, which had no tests before because no +/// fake existed. +/// +public sealed class ChatEngineTests +{ + private static readonly ModelInfo ModelA = new("model-a", "Model A", 8000, true); + private static readonly ModelInfo ModelB = new("model-b", "Model B", 8000, true); + + /// + /// Mirrors how the host starts up. ResumeAsync is what seeds the system prompt, so skipping it + /// would test an engine in a state the app never actually reaches. + /// + private static async Task<(ChatEngine Engine, FakeChatProvider Provider, TempAppPaths Paths)> BuildAsync( + params ModelInfo[] models) + { + var paths = new TempAppPaths(); + var provider = new FakeChatProvider { Models = models }; + var rotation = new RotationPolicy(paths); + var catalog = new JsonModelCatalog(paths, provider); + var sessions = new JsonSessionStore(paths); + var config = new JsonConfigStore(paths); + var engine = new ChatEngine(provider, rotation, catalog, sessions, config); + await engine.ResumeAsync(CancellationToken.None); + return (engine, provider, paths); + } + + private static async Task DrainAsync(ChatEngine engine, string text) + { + var sb = new System.Text.StringBuilder(); + await foreach (var chunk in engine.SendAsync(text, CancellationToken.None)) + { + if (chunk.IsAttemptRestart) sb.Clear(); + sb.Append(chunk.DeltaText); + } + return sb.ToString(); + } + + [Fact] + public async Task StreamsChunksAndPersistsTheTurn() + { + var (engine, provider, paths) = await BuildAsync(ModelA); + using var _ = paths; + provider.ThenSucceeds("Hello", " world"); + + var text = await DrainAsync(engine, "hi"); + + Assert.Equal("Hello world", text); + Assert.Equal(3, engine.Turns.Count); // system + user + assistant + Assert.Equal("Hello world", engine.Turns[^1].Content); + Assert.True(File.Exists(((IAppPaths)paths).SessionFile)); // default interface member + } + + [Fact] + public async Task PersistsTheTurnEvenWhenTheConsumerStopsAtTheFinalChunk() + { + // The natural way to consume this stream — and what the console host does — is to stop as + // soon as IsFinal arrives. That disposes the iterator at the yield, so any bookkeeping + // placed after it silently never runs: the reply was shown but never saved, and the + // model's success was never recorded. Draining to completion hides the bug, so this test + // deliberately breaks early. + var (engine, provider, paths) = await BuildAsync(ModelA); + using var _ = paths; + provider.ThenSucceeds("persisted"); + + await foreach (var chunk in engine.SendAsync("hi", CancellationToken.None)) + { + if (chunk.IsFinal) break; + } + + Assert.Equal(3, engine.Turns.Count); // system + user + assistant + Assert.Equal("persisted", engine.Turns[^1].Content); + Assert.True(File.Exists(((IAppPaths)paths).SessionFile)); + } + + [Fact] + public async Task RotatesToAnotherModelOnTransientFailure() + { + var (engine, provider, paths) = await BuildAsync(ModelA, ModelB); + using var _ = paths; + provider.ThenFails(ChatErrorKind.TransientRateLimit).ThenSucceeds("recovered"); + + var text = await DrainAsync(engine, "hi"); + + Assert.Equal("recovered", text); + Assert.Equal(new[] { "model-a", "model-b" }, provider.ModelsCalled); + } + + [Fact] + public async Task SignalsRestartSoPartialTextFromAFailedAttemptIsDiscarded() + { + var (engine, provider, paths) = await BuildAsync(ModelA, ModelB); + using var _ = paths; + + // First model emits real text, then dies. Without the restart signal the consumer would + // concatenate both attempts and show the answer twice. + provider.ThenFailsMidStream(ChatErrorKind.TransientServer, "The capital of France is Par") + .ThenSucceeds("The capital of France is Paris."); + + var sawRestart = false; + var sb = new System.Text.StringBuilder(); + await foreach (var chunk in engine.SendAsync("capital of france", CancellationToken.None)) + { + if (chunk.IsAttemptRestart) { sawRestart = true; sb.Clear(); continue; } + sb.Append(chunk.DeltaText); + } + + Assert.True(sawRestart); + Assert.Equal("The capital of France is Paris.", sb.ToString()); + Assert.Equal("The capital of France is Paris.", engine.Turns[^1].Content); + } + + [Fact] + public async Task DoesNotRotateWhenTheNetworkIsDown() + { + var (engine, provider, paths) = await BuildAsync(ModelA, ModelB); + using var _ = paths; + provider.ThenFails(ChatErrorKind.NetworkDown).ThenSucceeds("never reached"); + + var ex = await Assert.ThrowsAsync(() => DrainAsync(engine, "hi")); + + Assert.Equal(ChatErrorKind.NetworkDown, ex.Kind); + Assert.Single(provider.ModelsCalled); // stopped instead of burning every model + } + + [Fact] + public async Task DoesNotRotateOnAnInvalidRequest() + { + var (engine, provider, paths) = await BuildAsync(ModelA, ModelB); + using var _ = paths; + provider.ThenFails(ChatErrorKind.InvalidRequest).ThenSucceeds("never reached"); + + var ex = await Assert.ThrowsAsync(() => DrainAsync(engine, "hi")); + + Assert.Equal(ChatErrorKind.InvalidRequest, ex.Kind); + Assert.Single(provider.ModelsCalled); + } + + [Fact] + public async Task RemovesTheUserTurnWhenTheRequestFails() + { + var (engine, provider, paths) = await BuildAsync(ModelA); + using var _ = paths; + provider.ThenFails(ChatErrorKind.AuthFailure); + + await Assert.ThrowsAsync(() => DrainAsync(engine, "this must not stick")); + + // Only the system prompt survives; a failed turn must not be persisted on the next success. + Assert.Single(engine.Turns); + Assert.Equal(ChatMessage.SystemRole, engine.Turns[0].Role); + } + + [Fact] + public async Task RemovesTheUserTurnWhenCancelled() + { + var (engine, provider, paths) = await BuildAsync(ModelA); + using var _ = paths; + provider.ThenSucceeds("a", "b", "c"); + + using var cts = new CancellationTokenSource(); + await Assert.ThrowsAnyAsync(async () => + { + await foreach (var _chunk in engine.SendAsync("cancel me", cts.Token)) + { + cts.Cancel(); + } + }); + + Assert.Single(engine.Turns); + } + + [Fact] + public async Task SurfacesAFriendlyErrorWhenNoModelsAreAvailable() + { + // Previously RotationPolicy threw a bare InvalidOperationException here, which nothing + // upstream caught, so an empty model list crashed the app. + var (engine, _, paths) = await BuildAsync(); + using var __ = paths; + + var ex = await Assert.ThrowsAsync(() => DrainAsync(engine, "hi")); + + Assert.Contains("free models", ex.Message, StringComparison.OrdinalIgnoreCase); + } + + [Fact] + public async Task TreatsAStreamThatEndsWithoutAFinalChunkAsAFailure() + { + var (engine, provider, paths) = await BuildAsync(ModelA, ModelB); + using var _ = paths; + provider.ThenEndsWithoutFinalChunk("truncated").ThenSucceeds("complete"); + + var text = await DrainAsync(engine, "hi"); + + Assert.Equal("complete", text); + } + + [Fact] + public async Task PinnedModelIsPreferred() + { + var (engine, provider, paths) = await BuildAsync(ModelA, ModelB); + using var _ = paths; + engine.PreferredModelId = "model-b"; + provider.ThenSucceeds("ok"); + + await DrainAsync(engine, "hi"); + + Assert.Equal("model-b", provider.ModelsCalled[0]); + } +} diff --git a/tests/OpenKey.Core.Tests/ConfigStoreTests.cs b/tests/OpenKey.Core.Tests/ConfigStoreTests.cs new file mode 100644 index 0000000..f473078 --- /dev/null +++ b/tests/OpenKey.Core.Tests/ConfigStoreTests.cs @@ -0,0 +1,120 @@ +using OpenKey.Core.AppPaths; +using OpenKey.Core.Engine; +using OpenKey.Core.Storage; +using Xunit; + +namespace OpenKey.Core.Tests; + +public sealed class ConfigStoreTests +{ + [Fact] + public void MissingFileYieldsDefaults() + { + using var paths = new TempAppPaths(); + var config = new JsonConfigStore(paths).Load(); + + Assert.Equal(OpenKeyConfig.DefaultTheme, config.Theme); + Assert.Equal(OpenKeyConfig.DefaultMaxTokens, config.MaxTokens); + Assert.Null(config.PinnedModel); + } + + [Fact] + public void RoundTripsThroughDisk() + { + using var paths = new TempAppPaths(); + new JsonConfigStore(paths).Save( + OpenKeyConfig.Default.WithPinnedModel("vendor/model:free") with { Theme = "light" }); + + var reread = new JsonConfigStore(paths).Load(); + + Assert.Equal("vendor/model:free", reread.PinnedModel); + Assert.Equal("light", reread.Theme); + } + + [Fact] + public void PinningNullClearsThePreference() + { + var pinned = OpenKeyConfig.Default.WithPinnedModel("a"); + Assert.Equal("a", pinned.PinnedModel); + Assert.Null(pinned.WithPinnedModel(null).PinnedModel); + } + + [Fact] + public void HandEditedGarbageIsNormalisedRatherThanTrusted() + { + // Users are told they may edit this file, so every field is treated as untrusted. + using var paths = new TempAppPaths(); + File.WriteAllText(((IAppPaths)paths).ConfigFile, + """{"preferredModels":["good",""," "],"theme":" LIGHT ","maxTokens":-5}"""); + + var config = new JsonConfigStore(paths).Load(); + + Assert.Equal(new[] { "good" }, config.PreferredModels); + Assert.Equal("light", config.Theme); + Assert.Equal(OpenKeyConfig.DefaultMaxTokens, config.MaxTokens); + } + + [Fact] + public void CorruptFileIsQuarantinedAndDefaultsApply() + { + using var paths = new TempAppPaths(); + var file = ((IAppPaths)paths).ConfigFile; + File.WriteAllText(file, "not json at all"); + + var config = new JsonConfigStore(paths).Load(); + + Assert.Equal(OpenKeyConfig.DefaultTheme, config.Theme); + Assert.False(File.Exists(file)); + Assert.NotEmpty(Directory.GetFiles(paths.RootDir, "config.json.broken-*")); + } + + [Fact] + public async Task PinningAModelSurvivesARestart() + { + // The whole point of the config store: a pin used to last only until the app closed. + using var paths = new TempAppPaths(); + var provider = new FakeChatProvider { Models = new[] { new Providers.ModelInfo("m", "M", 8000, true) } }; + + var first = new ChatEngine( + provider, new RotationPolicy(paths), new JsonModelCatalog(paths, provider), + new JsonSessionStore(paths), new JsonConfigStore(paths)); + first.PreferredModelId = "vendor/pinned:free"; + + var second = new ChatEngine( + provider, new RotationPolicy(paths), new JsonModelCatalog(paths, provider), + new JsonSessionStore(paths), new JsonConfigStore(paths)); + await Task.CompletedTask; + + Assert.Equal("vendor/pinned:free", second.PreferredModelId); + } +} + +public sealed class TokenCounterTests +{ + [Fact] + public void HeuristicScalesWithLength() + { + var counter = new HeuristicTokenCounter(); + Assert.True(counter.Count(new string('x', 400)) > counter.Count(new string('x', 40))); + } + + [Fact] + public void EmptyTextCostsNothing() + { + Assert.Equal(0, new HeuristicTokenCounter().Count(string.Empty)); + } + + [Fact] + public void ConversationCountIncludesPerMessageOverhead() + { + ITokenCounter counter = new HeuristicTokenCounter(); + var messages = new[] + { + new Providers.ChatMessage(Providers.ChatMessage.UserRole, "hi"), + new Providers.ChatMessage(Providers.ChatMessage.AssistantRole, "hello"), + }; + + // Strictly more than the raw text alone: role framing is not free. + Assert.True(counter.Count(messages) > counter.Count("hi") + counter.Count("hello")); + } +} diff --git a/tests/OpenKey.Core.Tests/FakeChatProvider.cs b/tests/OpenKey.Core.Tests/FakeChatProvider.cs new file mode 100644 index 0000000..7e884c3 --- /dev/null +++ b/tests/OpenKey.Core.Tests/FakeChatProvider.cs @@ -0,0 +1,78 @@ +using System.Runtime.CompilerServices; +using OpenKey.Core.Providers; + +namespace OpenKey.Core.Tests; + +/// +/// Scriptable . Its absence is why — the +/// whole retry and rotation state machine — had no coverage at all. +/// +internal sealed class FakeChatProvider : IChatProvider +{ + private readonly Queue _attempts = new(); + + public string Id => "fake"; + public string DisplayName => "Fake"; + + public List ModelsCalled { get; } = new(); + public IReadOnlyList Models { get; set; } = Array.Empty(); + + private sealed record Attempt(IReadOnlyList Chunks, ChatException? Error, bool ErrorBeforeAnyChunk); + + /// Queues an attempt that streams the given text and finishes cleanly. + public FakeChatProvider ThenSucceeds(params string[] deltas) + { + var chunks = deltas.Select(d => new ChatChunk(d, false, null)).ToList(); + chunks.Add(new ChatChunk(string.Empty, true, "stop")); + _attempts.Enqueue(new Attempt(chunks, null, false)); + return this; + } + + /// Queues an attempt that streams text and then fails part-way, as a stall would. + public FakeChatProvider ThenFailsMidStream(ChatErrorKind kind, params string[] deltas) + { + var chunks = deltas.Select(d => new ChatChunk(d, false, null)).ToList(); + _attempts.Enqueue(new Attempt(chunks, new ChatException(kind, $"fake {kind}"), false)); + return this; + } + + /// Queues an attempt that fails before producing anything. + public FakeChatProvider ThenFails(ChatErrorKind kind) + { + _attempts.Enqueue(new Attempt(Array.Empty(), new ChatException(kind, $"fake {kind}"), true)); + return this; + } + + /// Queues an attempt whose stream ends with no final chunk at all. + public FakeChatProvider ThenEndsWithoutFinalChunk(params string[] deltas) + { + var chunks = deltas.Select(d => new ChatChunk(d, false, null)).ToList(); + _attempts.Enqueue(new Attempt(chunks, null, false)); + return this; + } + + public Task> ListModelsAsync(CancellationToken ct) => Task.FromResult(Models); + + public async IAsyncEnumerable StreamChatAsync( + ChatRequest request, + [EnumeratorCancellation] CancellationToken ct) + { + ModelsCalled.Add(request.Model); + + if (_attempts.Count == 0) + throw new ChatException(ChatErrorKind.TransientServer, "fake ran out of scripted attempts"); + + var attempt = _attempts.Dequeue(); + + if (attempt.ErrorBeforeAnyChunk && attempt.Error is not null) throw attempt.Error; + + foreach (var chunk in attempt.Chunks) + { + ct.ThrowIfCancellationRequested(); + await Task.Yield(); + yield return chunk; + } + + if (attempt.Error is not null) throw attempt.Error; + } +} diff --git a/tests/OpenKey.Core.Tests/OpenKey.Core.Tests.csproj b/tests/OpenKey.Core.Tests/OpenKey.Core.Tests.csproj index 4184973..b553c13 100644 --- a/tests/OpenKey.Core.Tests/OpenKey.Core.Tests.csproj +++ b/tests/OpenKey.Core.Tests/OpenKey.Core.Tests.csproj @@ -7,9 +7,9 @@ - - - + + + diff --git a/tests/OpenKey.Core.Tests/RollingWindowTests.cs b/tests/OpenKey.Core.Tests/RollingWindowTests.cs index e38879e..3a39acb 100644 --- a/tests/OpenKey.Core.Tests/RollingWindowTests.cs +++ b/tests/OpenKey.Core.Tests/RollingWindowTests.cs @@ -26,7 +26,6 @@ public void IsTransientCoversRetryableKinds() { Assert.True(ChatEngine.IsTransient(ChatErrorKind.TransientRateLimit)); Assert.True(ChatEngine.IsTransient(ChatErrorKind.TransientServer)); - Assert.True(ChatEngine.IsTransient(ChatErrorKind.NetworkDown)); Assert.True(ChatEngine.IsTransient(ChatErrorKind.MalformedResponse)); } @@ -36,4 +35,27 @@ public void IsTransientExcludesFatalKinds() Assert.False(ChatEngine.IsTransient(ChatErrorKind.AuthFailure)); Assert.False(ChatEngine.IsTransient(ChatErrorKind.QuotaExhausted)); } + + [Fact] + public void NetworkDownDoesNotRotate() + { + // With no route to the provider every model fails identically, so rotating would burn all + // five attempts and leave every model cooling down for an outage none of them caused. + Assert.False(ChatEngine.IsTransient(ChatErrorKind.NetworkDown)); + } + + [Fact] + public void NetworkDownIsNotBlamedOnTheModel() + { + Assert.False(ChatEngine.IsModelFault(ChatErrorKind.NetworkDown)); + Assert.True(ChatEngine.IsModelFault(ChatErrorKind.TransientRateLimit)); + Assert.True(ChatEngine.IsModelFault(ChatErrorKind.TransientServer)); + } + + [Fact] + public void InvalidRequestIsNotRetried() + { + // The request is the problem, not the model; an identical retry elsewhere cannot succeed. + Assert.False(ChatEngine.IsTransient(ChatErrorKind.InvalidRequest)); + } } diff --git a/tests/OpenKey.Core.Tests/StorageTests.cs b/tests/OpenKey.Core.Tests/StorageTests.cs new file mode 100644 index 0000000..6d7a25e --- /dev/null +++ b/tests/OpenKey.Core.Tests/StorageTests.cs @@ -0,0 +1,104 @@ +using OpenKey.Core.AppPaths; +using OpenKey.Core.Providers; +using OpenKey.Core.Storage; +using Xunit; + +namespace OpenKey.Core.Tests; + +public sealed class StorageTests +{ + [Fact] + public async Task SessionRoundTrips() + { + using var paths = new TempAppPaths(); + var store = new JsonSessionStore(paths); + + var snap = new SessionSnapshot("model-a", DateTimeOffset.UtcNow, new[] + { + new ChatMessage(ChatMessage.SystemRole, "sys"), + new ChatMessage(ChatMessage.UserRole, "hi"), + new ChatMessage(ChatMessage.AssistantRole, "hello — with an em-dash and 你好"), + }); + + await store.SaveAsync(snap, CancellationToken.None); + var loaded = await store.LoadAsync(CancellationToken.None); + + Assert.NotNull(loaded); + Assert.Equal("model-a", loaded!.ModelId); + Assert.Equal(3, loaded.Turns.Count); + Assert.Equal(snap.Turns[2].Content, loaded.Turns[2].Content); + } + + [Fact] + public async Task CorruptSessionIsQuarantinedRatherThanCrashing() + { + using var paths = new TempAppPaths(); + var store = new JsonSessionStore(paths); + var file = ((IAppPaths)paths).SessionFile; + await File.WriteAllTextAsync(file, "{ this is not json"); + + var loaded = await store.LoadAsync(CancellationToken.None); + + Assert.Null(loaded); + Assert.False(File.Exists(file)); + Assert.NotEmpty(Directory.GetFiles(paths.RootDir, "session.json.broken-*")); + } + + [Fact] + public async Task SavingIsBestEffortWhenTheTargetCannotBeWritten() + { + // A full disk or a read-only roaming profile used to crash the app after a reply had been + // generated but before it was shown. + using var paths = new TempAppPaths(); + var store = new JsonSessionStore(paths); + + // A directory where the session file belongs makes File.Create fail. + Directory.CreateDirectory(((IAppPaths)paths).SessionFile); + + var snap = new SessionSnapshot("m", DateTimeOffset.UtcNow, Array.Empty()); + await store.SaveAsync(snap, CancellationToken.None); // must not throw + } + + [Fact] + public async Task AnEmptyFreeModelListIsNeverCached() + { + // Caching an empty list pinned "no models" for the full 24h TTL, and because every launch + // then found a valid-but-empty cache the app stayed broken until %APPDATA% was deleted. + using var paths = new TempAppPaths(); + var provider = new FakeChatProvider + { + Models = new[] { new ModelInfo("paid", "Paid", 8000, false) }, + }; + var catalog = new JsonModelCatalog(paths, provider); + + await Assert.ThrowsAsync(() => catalog.RefreshAsync(CancellationToken.None)); + Assert.False(File.Exists(((IAppPaths)paths).ModelsCacheFile)); + } + + [Fact] + public async Task FreeModelsAreCachedAndReadBack() + { + using var paths = new TempAppPaths(); + var provider = new FakeChatProvider + { + Models = new[] + { + new ModelInfo("free-1", "Free One", 8000, true), + new ModelInfo("paid-1", "Paid One", 8000, false), + }, + }; + + var catalog = new JsonModelCatalog(paths, provider); + var models = await catalog.GetFreeModelsAsync(CancellationToken.None); + + Assert.Single(models); + Assert.Equal("free-1", models[0].Id); + Assert.True(File.Exists(((IAppPaths)paths).ModelsCacheFile)); + + // A second catalog over the same directory must read the cache rather than the provider. + var reread = await new JsonModelCatalog(paths, new FakeChatProvider()).GetFreeModelsAsync( + CancellationToken.None); + Assert.Single(reread); + Assert.Equal("free-1", reread[0].Id); + } +} diff --git a/tests/OpenKey.Tests/OpenKey.Tests.csproj b/tests/OpenKey.Tests/OpenKey.Tests.csproj index 646fdb8..8f3609d 100644 --- a/tests/OpenKey.Tests/OpenKey.Tests.csproj +++ b/tests/OpenKey.Tests/OpenKey.Tests.csproj @@ -8,9 +8,9 @@ - - - + + + diff --git a/tests/OpenKey.Tests/OpenRouterProviderTests.cs b/tests/OpenKey.Tests/OpenRouterProviderTests.cs new file mode 100644 index 0000000..e25ac60 --- /dev/null +++ b/tests/OpenKey.Tests/OpenRouterProviderTests.cs @@ -0,0 +1,195 @@ +using System.Net; +using System.Text; +using OpenKey.Core.Providers; +using OpenKey.Providers.OpenRouter; +using Xunit; + +namespace OpenKey.Tests; + +/// +/// Exercises the provider through its public surface with a stubbed transport, which covers SSE +/// framing, free-model detection and HTTP error mapping as the app actually uses them. +/// +public sealed class OpenRouterProviderTests +{ + private sealed class StubHandler : HttpMessageHandler + { + private readonly HttpStatusCode _status; + private readonly string _body; + private readonly string _contentType; + + public StubHandler(string body, HttpStatusCode status = HttpStatusCode.OK, string contentType = "application/json") + { + _body = body; + _status = status; + _contentType = contentType; + } + + protected override Task SendAsync(HttpRequestMessage request, CancellationToken ct) => + Task.FromResult(new HttpResponseMessage(_status) + { + Content = new StringContent(_body, Encoding.UTF8, _contentType), + }); + } + + private static OpenRouterProvider Provider(string body, HttpStatusCode status = HttpStatusCode.OK, string contentType = "application/json") => + new(new HttpClient(new StubHandler(body, status, contentType)) { Timeout = Timeout.InfiniteTimeSpan }, + () => "test-key"); + + private static async Task> CollectAsync(OpenRouterProvider provider) + { + var chunks = new List(); + var request = new ChatRequest("m", new[] { new ChatMessage(ChatMessage.UserRole, "hi") }); + await foreach (var c in provider.StreamChatAsync(request, CancellationToken.None)) chunks.Add(c); + return chunks; + } + + [Fact] + public async Task ParsesStreamedDeltas() + { + var sse = """ + data: {"choices":[{"delta":{"content":"Hel"}}]} + + data: {"choices":[{"delta":{"content":"lo"}}]} + + data: {"choices":[{"delta":{},"finish_reason":"stop"}]} + + data: [DONE] + + """; + + var chunks = await CollectAsync(Provider(sse)); + + Assert.Equal("Hello", string.Concat(chunks.Select(c => c.DeltaText))); + Assert.Contains(chunks, c => c.IsFinal && c.FinishReason == "stop"); + } + + [Fact] + public async Task TreatsDoneWithoutAFinishReasonAsACompletedReply() + { + // Some models close with [DONE] and never send finish_reason. Reporting null there made the + // engine discard a complete reply as malformed, cool the model down, and retry elsewhere. + var sse = """ + data: {"choices":[{"delta":{"content":"Complete answer"}}]} + + data: [DONE] + + """; + + var chunks = await CollectAsync(Provider(sse)); + + var final = Assert.Single(chunks.Where(c => c.IsFinal)); + Assert.Equal("stop", final.FinishReason); + } + + [Fact] + public async Task IgnoresCommentsAndBlankLines() + { + var sse = """ + : keep-alive + + data: {"choices":[{"delta":{"content":"x"}}]} + + data: [DONE] + + """; + + var chunks = await CollectAsync(Provider(sse)); + + Assert.Equal("x", string.Concat(chunks.Select(c => c.DeltaText))); + } + + [Fact] + public async Task CaptivePortalHtmlDoesNotCrash() + { + // A hotel or airport login page answers with HTML and HTTP 200. Unguarded this threw an + // unhandled JsonException during first run and the window closed on the stack trace. + var provider = Provider("Sign in to Wi-Fi", + contentType: "text/html"); + + var ex = await Assert.ThrowsAsync( + () => provider.ListModelsAsync(CancellationToken.None)); + + Assert.Equal(ChatErrorKind.NetworkDown, ex.Kind); + Assert.Contains("Wi-Fi", ex.Message, StringComparison.OrdinalIgnoreCase); + } + + [Fact] + public async Task MissingDataArrayIsReportedNotThrownRaw() + { + var ex = await Assert.ThrowsAsync( + () => Provider("""{"unexpected":true}""").ListModelsAsync(CancellationToken.None)); + + Assert.Equal(ChatErrorKind.MalformedResponse, ex.Kind); + } + + [Theory] + [InlineData(HttpStatusCode.Unauthorized, ChatErrorKind.AuthFailure)] + [InlineData(HttpStatusCode.Forbidden, ChatErrorKind.AuthFailure)] + [InlineData(HttpStatusCode.PaymentRequired, ChatErrorKind.QuotaExhausted)] + [InlineData(HttpStatusCode.TooManyRequests, ChatErrorKind.TransientRateLimit)] + [InlineData(HttpStatusCode.InternalServerError, ChatErrorKind.TransientServer)] + [InlineData(HttpStatusCode.ServiceUnavailable, ChatErrorKind.TransientServer)] + [InlineData(HttpStatusCode.BadRequest, ChatErrorKind.InvalidRequest)] + [InlineData(HttpStatusCode.NotFound, ChatErrorKind.InvalidRequest)] + [InlineData(HttpStatusCode.UnprocessableEntity, ChatErrorKind.InvalidRequest)] + public async Task MapsHttpStatusToErrorKind(HttpStatusCode status, ChatErrorKind expected) + { + var ex = await Assert.ThrowsAsync( + () => Provider("""{"error":{"message":"nope"}}""", status).ListModelsAsync(CancellationToken.None)); + + Assert.Equal(expected, ex.Kind); + } + + [Theory] + [InlineData("0", "0", true)] + [InlineData("0.0", "0.00", true)] + // The old exact string match classified this as paid, silently hiding a free model. + [InlineData("0.000000", "0.000000", true)] + [InlineData("0.0000015", "0", false)] + public async Task DetectsFreeModelsByParsingPriceNotMatchingText(string prompt, string completion, bool expectedFree) + { + var json = + "{\"data\":[{\"id\":\"vendor/model\",\"name\":\"Model\",\"context_length\":8000," + + $"\"pricing\":{{\"prompt\":\"{prompt}\",\"completion\":\"{completion}\"}}}}]}}"; + + var models = await Provider(json).ListModelsAsync(CancellationToken.None); + + Assert.Equal(expectedFree, Assert.Single(models).IsFree); + } + + [Fact] + public async Task TreatsTheFreeSuffixAsFreeRegardlessOfPricing() + { + var json = """{"data":[{"id":"vendor/model:free","name":"M","context_length":4096}]}"""; + + var models = await Provider(json).ListModelsAsync(CancellationToken.None); + + Assert.True(Assert.Single(models).IsFree); + } + + [Fact] + public async Task ReadsModelMetadata() + { + var json = """{"data":[{"id":"vendor/m","name":"Nice Name","context_length":32768,"pricing":{"prompt":"0","completion":"0"}}]}"""; + + var model = Assert.Single(await Provider(json).ListModelsAsync(CancellationToken.None)); + + Assert.Equal("vendor/m", model.Id); + Assert.Equal("Nice Name", model.DisplayName); + Assert.Equal(32768, model.ContextLength); + } + + [Fact] + public async Task SurfacesAnInStreamErrorObject() + { + var sse = """ + data: {"error":{"message":"upstream exploded"}} + + """; + + var ex = await Assert.ThrowsAsync(() => CollectAsync(Provider(sse))); + + Assert.Contains("upstream exploded", ex.Message, StringComparison.Ordinal); + } +} diff --git a/tests/OpenKey.Tests/TranscriptWriterTests.cs b/tests/OpenKey.Tests/TranscriptWriterTests.cs new file mode 100644 index 0000000..708343a --- /dev/null +++ b/tests/OpenKey.Tests/TranscriptWriterTests.cs @@ -0,0 +1,321 @@ +using System.Text; +using OpenKey.Ui; +using Spectre.Console; +using Xunit; + +namespace OpenKey.Tests; + +/// +/// Block-splitting behaviour of the streaming writer. The console here is redirected, so +/// ConsoleLayout.Rich is false and no raw text or cursor movement is emitted — what remains +/// is exactly the styled block output, which is what these assertions are about. +/// +public sealed class TranscriptWriterTests +{ + private static (TranscriptWriter Writer, StringWriter Output) Create() + { + var output = new StringWriter(); + var console = AnsiConsole.Create(new AnsiConsoleSettings + { + Ansi = AnsiSupport.No, + ColorSystem = ColorSystemSupport.NoColors, + Out = new AnsiConsoleOutput(output), + }); + return (new TranscriptWriter(console), output); + } + + [Fact] + public void RendersASingleParagraphOnComplete() + { + var (writer, output) = Create(); + writer.Append("Hello there."); + writer.Complete(); + + Assert.Contains("Hello there.", output.ToString(), StringComparison.Ordinal); + } + + [Fact] + public void SplitsParagraphsOnABlankLine() + { + var (writer, output) = Create(); + writer.Append("First paragraph.\n\nSecond paragraph."); + writer.Complete(); + + var text = output.ToString(); + Assert.Contains("First paragraph.", text, StringComparison.Ordinal); + Assert.Contains("Second paragraph.", text, StringComparison.Ordinal); + } + + [Fact] + public void KeepsAFenceWholeEvenWhenItContainsABlankLine() + { + // A blank line inside a code fence must not split the block, or the fence renders as two + // broken panels. + var (writer, output) = Create(); + writer.Append("```python\nfirst = 1\n\nsecond = 2\n```\n"); + writer.Complete(); + + var text = output.ToString(); + Assert.Contains("first = 1", text, StringComparison.Ordinal); + Assert.Contains("second = 2", text, StringComparison.Ordinal); + Assert.DoesNotContain("```", text, StringComparison.Ordinal); // rendered, not literal + } + + [Fact] + public void HandlesAFenceArrivingAcrossManyDeltas() + { + // The realistic case: a model emits a fence a few characters at a time. + var (writer, output) = Create(); + foreach (var piece in new[] { "Here:\n", "\n", "```", "js\n", "const ", "x = 1;\n", "```", "\n" }) + writer.Append(piece); + writer.Complete(); + + var text = output.ToString(); + Assert.Contains("Here:", text, StringComparison.Ordinal); + Assert.Contains("const x = 1;", text, StringComparison.Ordinal); + Assert.DoesNotContain("```", text, StringComparison.Ordinal); + } + + [Theory] + [InlineData(true)] + [InlineData(false)] + public void NeverShowsRawBackticksWhateverTheConsoleSupports(bool ansi) + { + // A single delta can carry both fence markers, which nets to "not inside a fence". That + // used to let a whole code block reach the screen as raw markdown before the flush + // replaced it — and it only reproduced on consoles that allow raw streaming, so it passed + // locally and failed in CI. + var output = new StringWriter(); + var console = AnsiConsole.Create(new AnsiConsoleSettings + { + Ansi = ansi ? AnsiSupport.Yes : AnsiSupport.No, + ColorSystem = ColorSystemSupport.NoColors, + Out = new AnsiConsoleOutput(output), + }); + + var writer = new TranscriptWriter(console); + writer.Append("```python\nfirst = 1\n\nsecond = 2\n```\n"); + writer.Complete(); + + Assert.DoesNotContain("```", output.ToString(), StringComparison.Ordinal); + } + + [Fact] + public void RendersAnUnterminatedFenceOnComplete() + { + // A stream cut short mid-fence must still show what arrived rather than swallowing it. + var (writer, output) = Create(); + writer.Append("```python\nprint(1)\n"); + writer.Complete(); + + Assert.Contains("print(1)", output.ToString(), StringComparison.Ordinal); + } + + [Fact] + public void SplitsWhenABlankLineArrivesAcrossTwoDeltas() + { + var (writer, output) = Create(); + writer.Append("One.\n"); + writer.Append("\nTwo."); + writer.Complete(); + + var text = output.ToString(); + Assert.Contains("One.", text, StringComparison.Ordinal); + Assert.Contains("Two.", text, StringComparison.Ordinal); + } + + [Fact] + public void ResetDiscardsEverythingFromAnAbandonedAttempt() + { + // What stops a mid-reply rotation from rendering the answer twice concatenated. + var (writer, output) = Create(); + writer.Append("The capital of France is Par"); + writer.Reset(); + writer.Append("The capital of France is Paris."); + writer.Complete(); + + var text = output.ToString(); + Assert.Contains("The capital of France is Paris.", text, StringComparison.Ordinal); + + // How "discarded" is observable depends on whether the console allows raw streaming, and + // a StringWriter has no cursor for an escape sequence to move. Assert what is actually + // true in each case rather than picking one and hoping the environment agrees — an + // earlier version of this test passed locally and failed in CI for exactly that reason. + if (output.ToString().Contains('')) + Assert.Contains("[", text, StringComparison.Ordinal); // an erase was issued + else + Assert.Equal(1, CountOccurrences(text, "The capital of France")); + } + + [Fact] + public void RewindIssuesAnEraseRatherThanClearingScrollback() + { + // The rewind path only runs on an ANSI console, so nothing else in this suite reaches it. + // EraseInDisplay(2) or ClearScrollback here would wipe the conversation — the precise + // reason LiveDisplay was rejected — so pin the sequence that is allowed. + var output = new StringWriter(); + var console = AnsiConsole.Create(new AnsiConsoleSettings + { + Ansi = AnsiSupport.Yes, + ColorSystem = ColorSystemSupport.NoColors, + Out = new AnsiConsoleOutput(output), + }); + + var writer = new TranscriptWriter(console); + writer.Append("some text that will be replaced\n\n"); + writer.Complete(); + + var text = output.ToString(); + Assert.DoesNotContain("", text, StringComparison.Ordinal); // never whole-screen + Assert.DoesNotContain("", text, StringComparison.Ordinal); // never scrollback + } + + [Fact] + public void EmitsNothingForEmptyInput() + { + var (writer, output) = Create(); + writer.Complete(); + + Assert.True(string.IsNullOrWhiteSpace(output.ToString())); + } + + private static int CountOccurrences(string haystack, string needle) + { + var count = 0; + var i = 0; + while ((i = haystack.IndexOf(needle, i, StringComparison.Ordinal)) >= 0) + { + count++; + i += needle.Length; + } + return count; + } +} + +public sealed class MarkdownRenderingTests +{ + private static string Render(string markdown) + { + var output = new StringWriter(); + var console = AnsiConsole.Create(new AnsiConsoleSettings + { + Ansi = AnsiSupport.No, + ColorSystem = ColorSystemSupport.NoColors, + Out = new AnsiConsoleOutput(output), + }); + MarkdownConsoleRenderer.Render(console, markdown); + return output.ToString(); + } + + [Fact] + public void KeepsLinksWhoseUrlContainsBrackets() + { + // The old code filtered out any URL containing a bracket, silently dropping the target. + // Brackets are legal in URLs and common in generated ones. + var text = Render("See [the docs](https://example.com/a[b]c) for more."); + + Assert.Contains("the docs", text, StringComparison.Ordinal); + Assert.Contains("for more.", text, StringComparison.Ordinal); + } + + [Fact] + public void ModelOutputCannotInjectConsoleMarkup() + { + var text = Render("A reply containing [red]alarming[/] markup and [[brackets]]."); + + Assert.Contains("alarming", text, StringComparison.Ordinal); + Assert.Contains("[red]", text, StringComparison.Ordinal); // shown literally, not applied + } + + [Fact] + public void RendersInlineCodeWithoutABackground() + { + Assert.Contains("value", Render("Set `value` first."), StringComparison.Ordinal); + } +} + +public sealed class ThemeTests +{ + [Theory] + [InlineData("default")] + [InlineData("dark")] + [InlineData("light")] + [InlineData("mono")] + public void EveryAdvertisedThemeApplies(string name) + { + Assert.True(Theme.IsKnown(name)); + Theme.Apply(name); + Assert.Equal(name, Theme.Current); + Assert.False(string.IsNullOrWhiteSpace(Theme.Brand)); + Theme.Apply("default"); + } + + [Fact] + public void UnknownThemesAreRejectedRatherThanSilentlyAccepted() + { + Assert.False(Theme.IsKnown("neon")); + } + + [Fact] + public void MonoRemovesHueWithoutRemovingMeaning() + { + // Accessibility check: severity must never be carried by colour alone. Under mono there is + // no hue left, so if this palette is still usable the glyphs and titles are doing the work. + Theme.Apply("mono"); + foreach (var style in new[] { Theme.Ok, Theme.Warn, Theme.Danger, Theme.Brand }) + { + Assert.DoesNotContain("red", style, StringComparison.Ordinal); + Assert.DoesNotContain("green", style, StringComparison.Ordinal); + Assert.DoesNotContain("yellow", style, StringComparison.Ordinal); + } + Theme.Apply("default"); + } + + [Fact] + public void SeverityGlyphsDifferSoRedGreenConfusionIsNeverTheOnlySignal() + { + Assert.NotEqual(Glyphs.Ok, Glyphs.Fail); + } +} + +public sealed class TextWidthTests +{ + [Fact] + public void AsciiIsOneCellPerCharacter() + { + Assert.Equal(5, TextWidth.Of("hello")); + } + + [Fact] + public void CjkIsTwoCellsPerCharacter() + { + Assert.Equal(4, TextWidth.Of("你好")); + } + + [Fact] + public void CombiningMarksTakeNoSpace() + { + // "e" + combining acute renders in one cell, not two. + Assert.Equal(1, TextWidth.Of("é")); + } + + [Fact] + public void EmojiIsTwoCells() + { + Assert.Equal(2, TextWidth.Of("\U0001F600")); + } + + [Fact] + public void SurrogatePairsCountAsOneGlyphNotTwoChars() + { + var emoji = "\U0001F600"; + Assert.Equal(2, emoji.Length); // two chars + Assert.Equal(2, TextWidth.Of(emoji)); // but two cells, not four + } + + [Fact] + public void MixedTextAddsUp() + { + Assert.Equal(5 + 4, TextWidth.Of("hello你好")); + } +}