Repository navigation
Correct docs that disagreed with the code (docs accuracy pass) - #330
Conversation
Twenty-one corrections across the README and nine docs, each checked against the code: where Windows keeps logs, models and the web view profile, which dashboard endpoints need the admin role, init's consent for the copilot CLI, the size cut-off for tier 1, button labels and the Activity page's limit. Co-Authored-By: Claude <noreply@anthropic.com>
siebe41
left a comment
There was a problem hiding this comment.
Needs changes, small
All 20 edits in this PR match the code on c67c91b, and the PR changes no code (all 10 files are .md). The problem is that three of the corrected statements survive, still wrong, in other docs the pass was meant to cover. The worst is docs/clients.md, the page the README sends readers to for client setup.
Findings
Medium: docs/clients.md:23 still says the gateway key is never read.
- The line is
export ANTHROPIC_AUTH_TOKEN=semantic-router # any value: the router never reads or forwards it. This is the statement the PR fixed atREADME.md:299. GatewayAuthentication.Admit(GatewayGuard.cs:95-132) reads that token as a gateway key.docs/architecture.md:194says "the router never reads or forwards credentials of a client".- The dashboard's Connect clients snippet says the same thing in code (
wwwroot/js/config.js:447, "the router ignores it and never forwards it"). - Fix: correct clients.md the same way as the README. Note the config.js text as a follow-up, since this PR is docs-only.
Medium: docs/host.md:55 still says the background service "restarts if it fails", for every platform.
- The table at
host.md:73says Windows does "not automatically", and the PR fixed the same claim indesktop.md:125. - The tool and the app tell Windows users the same thing:
ServiceCommand.cs:97, printed byservice installon every platform;WelcomeWindow.cs:39andSettingsWindow.cs:59;- the dashboard's
wwwroot/js/service.js:229; - the doc comment at
Desktop/RouterService.cs:7.
- Fix: correct host.md:55 here, and file a follow-up for the code strings.
Low: docs/architecture.md:115 still says "semantic-router.db next to the configuration". On Windows with the default directory, the database is in %LOCALAPPDATA%\semantic-router (LocalData.StateFile, RouterHost.cs:97). This is the same stale claim the PR fixed in desktop.md and network.md.
Low: docs/desktop.md:74 and :174 say only "the database and logs" move to the local data directory.
LocalData.Foldersalso movesmodels/andwebview/, ashost.md:125lists.- In the uninstall paragraph (
:74) this matters. On Windows, two folders stay behind after uninstall, but the sentence reads as if the configuration directory is the only one to delete. - Suggested wording: "…the configuration directory and, on Windows with the default directory, the local data directory
%LOCALAPPDATA%\semantic-router(state database, models, logs, web view profile) stay; delete them yourself…".
Low: the init consent wording the PR fixed in the README survives in two places.
docs/TERMS.md:139says "initasks before enabling each subscription CLI it finds".docs/host.md:45says "asking before it enables a subscription CLI …--yesenables them without asking".- Copilot is never asked about, and
--yesdoes not enable it (InitCommand.cs:294-300,:335-340). - host.md:45 links to the section that explains this, so it is borderline. TERMS.md, though, is the page people read for work use, so it should be exact.
- The tool's own help text has the same imprecision (
Program.cs:70); its--yesline (:97) is already right.
Low: docs/routing-quality.md:258 leaves POST api/reliability/reset out of the admin-only sentence. The endpoint is in the table two lines above (:256) and carries DashboardWriteMetadata (ReliabilityApi.cs:30). The PR added it to dashboard.md's admin list, but not here.
nit: README.md:299 is still slightly imprecise. With gateway keys set, a client on this machine is still admitted with any value unless Middleware:RequireGatewayKeyForLocalClients is on (off by default; GatewayGuard.cs:98). The key is only required from other machines. The new wording is safe advice, just stricter than the code. A more exact version: "any value from this machine; with gateway keys, send one (required from other machines)".
nit: README.md:321 describes the Service page as "(the background service)". The page also covers how this router runs (Restart, Stop) and who can connect (dashboard.md:50). "Service (how the router runs, who can connect, the background service)", or just "Service", would be closer.
nit: docs/observability.md:124 says "records on the same meter". SqliteRouterState creates its own new Meter(SemanticRouterDiagnostics.MeterName) (SqliteRouterState.cs:58). That is the same meter name (SemanticRouter), not the same instance. AddMeter("SemanticRouter") picks it up either way, so "under the same meter name" would be exact.
Checked
Every edit was checked against the code on c67c91b, and all are right:
- Windows service logs go to the local data directory (
ServiceCommand.cs:387,RouterLog.FileOf). - Size tiers are ≤ 9.5B: 1 in host.md and providers.md (
ModelLineup.cs:383), and init uses the sameModelLineup.Suggest. - ONNX models go to
LocalData.ModelsDirectoryin the tool (RouterHost.cs:95). - Memory page button labels match
memory.js:135-137. - README init consent: claude and codex are asked about, copilot is added disabled unless
--enable-clinames it, and--no-cliadds none (InitCommand.cs:288-345). - Dashboard remote access:
RequireLocalClientdefaults to true and refuses remote callers before any token is checked (DashboardAccess.cs:217-224). - Admin rail: Service, JSON, Audit log (
core.js:110). - Route health is admin-only (
EvaluationApi.cs:171). - Dashboard admin list: dashboard.md:204 now names every endpoint marked
DashboardWriteMetadataorDashboardAdminReadMetadata. RouterDoctoris in AspNetCore andDoctorCommandis in Hosting.- The Windows sign-in entry has no restart: the app only polls (
DesktopRouter.Refresh). - desktop.md "Where things live":
logs/andwebview/are in the local data directory, anddesktop.jsonstays with the configuration. - Activity shows the last 200 (
shell.js:28,132). Live events can briefly push the in-page list up to 500 (live.js:74) until the next refresh, which is probably where the old number came from. dropped_writesis the only metric not defined onSemanticRouterDiagnostics.- Links: all
#anchorlinks in the changed docs resolve against GitHub-style heading slugs. - CHANGELOG: no line is needed, because CONTRIBUTING.md#changelog exempts documentation-only changes.
CI is green and the PR doesn't conflict with main. Nothing pushed.
…d, Windows does not restart the service, the database is in the local data directory, init never asks about copilot, reliability reset is admin-only clients.md and architecture.md: a client's key is checked against the gateway keys (any value from this machine), never forwarded. host.md: restarts on Linux and macOS only. architecture.md: semantic-router.db in the local data directory. desktop.md: models and webview move too, and the uninstall paragraph names both directories. TERMS.md and host.md: claude and codex are asked about; copilot is added disabled unless --enable-cli names it. routing-quality.md: reliability reset needs the admin role. README: gateway key and Service page wording. observability.md: same meter name. Code strings filed as #339.
|
Review addressed. I merged
Also: the Verified: |
What changed
A docs accuracy pass: I checked every command, flag, configuration key, endpoint, header, default, button label and cross-doc link in
README.mdanddocs/*.mdagainst the code. 17 statements disagreed with it (20 edits), across the README and nine docs. Each is fixed in the doc. No code changed.%LOCALAPPDATA%\semantic-routerwith the default directory)ServiceCommand.cs:387passesLocalDataDirectory;ServicePlatforms.cs:434logs thereinitadds)ModelLineup.cs:383<= 9.5 => 1models/{name}into the configuration directoryRouterHost.cs:95→LocalData.ModelsDirectory=LocalDataDirectory/modelswwwroot/js/memory.js:135-137initasks before enabling each subscription CLI;--yesenables them--enable-clinames itInitCommand.cs:294-300(copilotAsk = false),:335-340;Program.cs:97help textANTHROPIC_AUTH_TOKEN: "the router never reads or forwards it"GatewayGuard.cs:95-132reads it as a gateway keyRequireLocalClientis off and a token is setDashboardAccess.cs:217-224;RequireLocalClientdefaults to truewwwroot/js/core.js:110EvaluationApi.cs:171DashboardWriteMetadata;DashboardAccess.csadmits write metadata for Admin onlydoctor(RouterDoctor) in HostingRouterDoctoris in AspNetCore; Hosting has the commandAspNetCore/Diagnostics/RouterDoctor.cs:32;Hosting/DoctorCommand.cs:41ServicePlatforms.csWindowsSignInServiceis a Run-key value; host.md's table says "not automatically"webview/included, in the configuration directorywebview/and the database are in%LOCALAPPDATA%\semantic-routerDesktop/App.cs:549,RouterLog.cs:15,HostSettings.LocalDataDirectoryForwwwroot/js/shell.js:28,132loadactivity?limit=200on start and every refreshPOST api/reliability/reset,POST api/config/template-defaults/use/keep,GET api/routes/health,POST api/prices/lookup("everything else is open to the read-only role")DashboardWriteMetadata(ReliabilityApi.cs:30,TemplateDefaultsApi.cs:46,57,EvaluationApi.cs:171,RouterDashboardEndpoints.cs:506);DashboardAccess.cs:353refuses them to non-adminsSemanticRouterDiagnostics"semantic_router.state.dropped_writes, from Storage.SqliteSqliteRouterState.cs:60Review follow-up (0bb325d)
The review found the same stale claims surviving in other docs. Fixed:
ANTHROPIC_AUTH_TOKEN: "the router never reads or forwards it"semantic-router.dbnext to the configuration%LOCALAPPDATA%\semantic-routeron Windows by default)init)--yesenables them--yes, unless--enable-clinames itSemanticRouter)"Why
These docs are what users configure the router from, and before 1.0 they should describe what the code does. Most of the errors were stale after a change elsewhere:
How it was verified
deploy/scripts, and anchors.dotnet build,dotnet test,dotnet formatanddotnet packare unaffected and were not run. The Pages workflow builds the site with VitePress's dead-link check on this PR.npm ci+npm run buildinsite/(VitePress, with its dead-link check) completes; no test asserts the changed wording (searchedtests/).Not done
service install, the desktop app and the dashboard; the Connect clients gateway-key comment;init --help's consent line; theServicePlatforms.cs:406log comment) are filed as Service restart, gateway key and init consent claims the docs corrected are still in the tool, app and dashboard text #339, since this PR is docs-only.tools/check-egress-hosts.pymakes network calls).docs/host.md's release-to-schema table could not be tied to tags (CHANGELOG has no 0.5.0 section). It is consistent withSchemaVersion = 4.Sprint task O03 (overflow); no issue tracks it.