docs: apply Copilot review feedback on PR #418

Three docstring + CHANGELOG drift items from the post-review
M3 + Mi1 fixes:

- ``make_list_handler`` docstring referenced ``cfg.list_resolve_title``
  (singular) but the field renamed to ``list_resolve_titles``
  (bulk variant) when the N+1 fix landed. Updated to the plural
  name + a one-line note about the bulk SELECT pattern.
- ``make_saved_handler`` docstring still claimed kind was derived
  from ``cfg.audit_action_prefix`` string-compare. The Mi1 fix
  replaced that with the explicit ``cfg.list_kind`` field +
  fail-loud-on-missing semantic; docstring now describes the
  current contract.
- CHANGELOG ``[Unreleased]`` entry said "Three new
  ``SessionEndpointConfig`` fields" and listed the singular
  ``list_resolve_title`` wired to ``get_workstream_display_name``.
  Updated to "Four" + the bulk plural names + the new
  ``list_kind`` field with its rationale (distinct from
  ``audit_action_prefix``; fail-loud on misconfig).

The fourth review comment — code-quality bot flagging the ``...``
ellipsis body on the new ``get_workstream_display_names`` Protocol
method as "statement has no effect" — is a false positive.
``...`` is the canonical Protocol method body throughout
``turnstone/core/storage/_protocol.py`` (every other method uses
it). Refuting; the file's pattern wins over the bot's per-method
suggestion.

No code changes; docstring + CHANGELOG only. Tests + lint + mypy
unchanged.
This commit is contained in:
Patrick Buckley
2026-04-26 13:10:05 -07:00
committed by Patrick Buckley
parent edf52016ac
commit e8a6b0632d
2 changed files with 27 additions and 11 deletions
+15 -5
View File
@@ -22,14 +22,24 @@ Three release tracks are maintained:
``GET /v1/api/workstreams`` + ``GET /v1/api/workstreams/saved``
and coord ``GET /v1/api/workstreams`` + ``GET /v1/api/workstreams/saved``
handlers now share two factory bodies via
``make_list_handler(cfg)`` and ``make_saved_handler(cfg)``. Three
``make_list_handler(cfg)`` and ``make_saved_handler(cfg)``. Four
new ``SessionEndpointConfig`` fields capture the per-kind
divergence:
- ``list_resolve_title: ListResolveTitle | None`` — interactive
wires :func:`turnstone.core.memory.get_workstream_display_name`
so user-set aliases override the auto-generated ws-XXXX name in
the active list. Coord wires ``None`` (no alias surface).
- ``list_resolve_titles: ListResolveTitles | None`` — interactive
wires :func:`turnstone.core.memory.get_workstream_display_names`
(new bulk helper added on the storage layer + ``memory.py``)
so the active-list endpoint resolves every user-set alias in
ONE ``SELECT ... WHERE ws_id IN (...)`` instead of the pre-lift
per-row N+1. Coord wires ``None`` (no alias surface today).
- ``list_kind: WorkstreamKind | None`` — required storage-side
kind classifier passed to ``list_workstreams_with_history``.
Interactive wires ``WorkstreamKind.INTERACTIVE``; coord wires
``WorkstreamKind.COORDINATOR``. Distinct from
``audit_action_prefix`` (audit-action namespacing) so adding a
third kind doesn't have to overload the audit prefix as a
classifier; missing value surfaces as 500 with a clear log
line rather than silently filtering for the wrong kind.
- ``saved_state_filter: str | None`` — coord wires ``"closed"``
so only explicitly-closed coordinators surface in the
saved-card grid. Interactive wires ``None`` (the storage
+12 -6
View File
@@ -1886,8 +1886,10 @@ def make_list_handler(cfg: SessionEndpointConfig) -> Handler:
interactive ``None`` (auth middleware covers it).
- ``cfg.manager_lookup`` — already used by every other lifted
verb.
- ``cfg.list_resolve_title`` — interactive's user-alias override;
coord ``None``.
- ``cfg.list_resolve_titles`` — interactive's bulk user-alias
lookup; coord ``None``. Single ``SELECT ... WHERE ws_id IN
(...)`` resolves every active row's title in one storage
round-trip (replaces the pre-lift per-row N+1).
Always-include row shape: ``{ws_id, name, state, kind,
parent_ws_id, user_id}``. SDK consumers don't branch on kind.
@@ -1976,10 +1978,14 @@ def make_saved_handler(cfg: SessionEndpointConfig) -> Handler:
Per-kind divergence:
- ``cfg.permission_gate`` — coord's ``admin.coordinator`` check.
- ``cfg.audit_action_prefix`` — used to derive the kind filter
("workstream" → INTERACTIVE; "coordinator" → COORDINATOR).
Already wired on every endpoint cfg; reuse here keeps the
cfg surface tight.
- ``cfg.list_kind`` — required ``WorkstreamKind`` passed straight
through to ``list_workstreams_with_history(kind=...)``. The
handler treats a missing value as a configuration error and
surfaces 500 with a clear log line — fails loud rather than
silently filtering for the wrong kind. Distinct from
``audit_action_prefix`` (audit-action namespacing) so adding a
third kind doesn't have to overload the audit prefix as a
kind classifier.
- ``cfg.saved_state_filter`` — coord wires ``"closed"`` so only
explicitly-closed coordinators surface; interactive wires
``None`` (any state except the tombstoned ``deleted`` rows the