Skip to content

Take session command arguments as one object #560

Closed

Description

@alex-rawlings-yyc

Session edit commands take changeId as a trailing positional ?: ChangeId | null, after other trailing optionals such as absorbedPhraseId and adoptStaleAnalysisId. A caller has to pass null placeholders to reach it, since JSON-RPC sends a skipped positional argument as null, and handlers convert back with ?? undefined.

Move commands to one args object, e.g. { sessionId, changeId?, ...intentArgs }. Every command in the repo is positional today, so this changes a convention rather than one family.

Done when: session commands take one args object, and the decision is made before 4a writes the WebView's callers.

Raised in review of #527.

Activity

  1. imnasnainaec commented on Oct 29, 2026

    @imnasnainaec
    Contributor

    Recommendation: take (sessionId, args), where args is { changeId?, ...intentArgs }. Make args required on every session command, including ones with no intent arguments like undo and redo, which take { changeId? }.

    Why change. The trailing optionals force callers to pass null placeholders, and handlers turn them back into undefined with ?? undefined. A field that's left out of an object needs neither.

    Why not one object holding everything. paranext-core keeps identifiers positional and puts optional settings in an object, for example platformScriptureEditor.openScriptureEditor(projectId?, options?, existingTabIdToReplace?). None of the core commands I checked puts the identifier inside a single args object. Keeping sessionId positional stays close to that convention and still removes the placeholders.

    Why it matters for #462. Core's cross-process rule (.claude/rules/architecture/csharp-patterns.md, "Cross-process parameter alignment") says StreamJsonRpc matches a C# handler by method name and argument count. TS optional parameters change how many arguments are actually sent, and a wrong count fails silently with -32602. With a required args, every call sends exactly two arguments, which map to a C# handler of (string sessionId, TRequest args). Adding a field to a request later doesn't change the argument count.

    Timing: after #527, #528 and #529 merge, and before #453 writes the WebView's callers.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Labels

No labels
No labels

Type

No type

Projects

No projects

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions