Skip to content

Describe the read model argument as an identifier - #144

Open
woksin wants to merge 1 commit into
mainfrom
fix/read-model-argument-help-text
Open

woksin wants to merge 1 commit into
mainfrom
fix/read-model-argument-help-text

Conversation

@woksin

@woksin woksin commented Sep 13, 2026

Copy link
Copy Markdown
Collaborator

Summary

read-models get, snapshots and instances send their positional argument as the read model identifier, but the help text called it a container name and pointed at cratis read-models list — which prints both an Identifier and a Container column.

A user following the help picked the wrong column and the server answered with an unhandled error rather than a useful message.

No tracking issue exists; found capturing CLI output with 3.1.6 against Chronicle 18.1.6.

Fixed

  • The <READ_MODEL> argument is described as the read model identifier and names the column to read it from, matching what the commands actually send.

Notes for review

  • No behaviour change: the wire value was always ReadModelIdentifier. This aligns the [Description], the [LlmOption] metadata and the settings doc comment with it.
  • The server side of the same mistake is fixed separately in Cratis/Chronicle fix/read-model-lookup-miss-guard, which turns the resulting NullReferenceException into a meaningful error. The two are independent; either can merge first.
  • GetReadModelInstancesCommand catches an RPC error whose detail contains "NullReferenceException" and reports it as "read model is client-owned". That was papering over the server defect above and is the wrong explanation for an unknown identifier — worth re-scoping once the Chronicle fix ships. Left untouched here.
  • read-models occurrences uses a different argument name (<READ_MODEL_TYPE>) and may be worth a consistency pass.

Verification

  • New spec asserts the description says identifier, does not say container name, and names the Identifier column that ListReadModelsCommand prints — so help text and wire payload cannot drift apart again.
  • Cli.Specs: 1,111 tests passing.

`read-models get|snapshots|instances` send their positional argument as the
read model identifier, but the help text called it a container name and
pointed at 'cratis read-models list' - which prints both an Identifier and a
Container column. A user following the help picked the wrong column, and the
server answered with an unhandled error rather than a useful message.

Align the argument descriptions, the LlmOption metadata and the settings doc
comment with what is actually sent, and name the column to read it from.
No behaviour change; the wire value was always the identifier.

Adds a spec so the help text and the wire payload cannot drift apart again.
@woksin woksin added the patch label Sep 13, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant