Skip to content

Deferred review backlog rollup (PRs #10, #13, #14) — Medium + Low #18

Description

@Anarchid

Rollup of deferred /roast and greptile items across recent merged PRs that haven't been picked up in follow-up commits. The top High item (handleError context heuristic missing status guard) is tracked separately as #17.

Medium

PR #14 — beforeRequest hook in streaming paths

  • beforeRequest hook receives stale normalizedRequest across tool-continuation rounds. Each iteration of the while (toolDepth <= maxToolDepth) loop calls streamOnce(..., { normalizedRequest: request }) with the original request, while providerRequest is rebuilt as a continuation. Anyone keying off the normalized side will think they're getting one request N times. Either document the semantics on MembraneHooks.beforeRequest in src/types/config.ts, or rebuild normalizedRequest per continuation round. Roast addressed 5 of 6 items in follow-up b21636d but skipped this cross-file doc task — appears to be an "I'm editing this file anyway" bias.

PR #13 — participant names in native tool streaming

  • buildNativeToolRequest() hardcodes the participant-name prefix as ${msg.participant}: at src/membrane.ts:986, while NativeFormatter exposes a configurable nameFormat (default '{name}: '). Any caller constructing Membrane with a custom nameFormat (e.g. [{name}] or @{name}: ) silently gets a different prefix on the streaming-native path than on the formatter path. Stays consistent today only for the default. Greptile P2.

Low

PR #13

  • Name-prefix applied to every text block in multi-text messages rather than only the first, so a message with two text blocks becomes "Alice: Hello" + "Alice: World". Mirrors a pre-existing wart in NativeFormatter.convertContent(). Guard with a "first text block only" check.

PR #10 — provider bug fixes

  • No-op ternary on system: ... at two sites. src/membrane.ts:1274 and :1344 (buildContinuationRequest and buildContinuationRequestWithImages). Both branches of the inner Array.isArray(...) && length > 0 ? prefillResult.systemContent : prefillResult.systemContent return the same value. The falsy branch should be undefined to actually filter empty arrays.
  • OpenRouterAdapter.messageToContent() drops image_url blocks. src/providers/openrouter.ts:546-560 — handles the new array-content shape but only extracts text, silently dropping image_url blocks that the OpenRouterContentBlock type union allows. The adapter already emits image_url blocks when converting from Anthropic image content (lines 398-407), so they're a real possibility on response. Either handle them or document why responses can't have them.

Origin

Audit run 2026-05-21 against the GitHub-visible review surface. Membrane is the only repo in the ecosystem where greptile is wired up (good); items here are the ones greptile flagged where the PR author didn't reply inline (typical pattern: when the author replies acknowledging, items get fixed; when they're in the "Comments Outside Diff" / "Prompt To Fix All" block with no inline reply, they get silently merged past).

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions