fix(fb-33): address PR #124 round 1 review

- pipeline: refresh module docstring step 5 to reflect new cancel
  semantics (RetrievalDone/Token/Final + LlmStreamAborted)
- wire schema: spell out refusal-path behavior in answer_event.v1
  description (only retrieval_done emitted; no final)
- test: factual comment on relax_score_gate-using test corrected
- test: new Ollama-gated stream_score_gate_refusal_emits_only_retrieval_done
- test: new ask_emits_no_final_when_cancelled_mid_stream pinning
  the no-Final invariant on cancel
- pipeline: large_enum_variant comment broadened to acknowledge
  RetrievalDone.hits as the dominant per-emit cost
- HOTFIXES: log AskOpts.stream_sink internal API break per spec
  contract policy

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

crates/kebab-rag/src/pipeline.rs

@@ -12,9 +12,12 @@
 //!    ~4 chars / token, matching the kb-chunk convention).
 //! 4. Render the `rag-v1` prompt (system + user) verbatim per design.
 //! 5. Generate via `LanguageModel::generate_stream`. The token loop runs
-//!    on the calling thread; `opts.stream_sink` (if any) gets each
-//!    token forwarded synchronously and a dropped receiver does not
-//!    abort generation.
+//!    on the calling thread; `opts.stream_sink` (if any) emits
+//!    `StreamEvent::RetrievalDone` once after retrieve+stale-stamp,
+//!    `StreamEvent::Token` per LM chunk, and `StreamEvent::Final` on
+//!    success. A dropped receiver triggers cancel: SendError on Token
+//!    breaks the LM loop + records `RefusalReason::LlmStreamAborted`
+//!    in the persisted Answer (p9-fb-33).
 //! 6. Citation extract — STRICT regex `\[#(\d{1,3})\]`, no false
 //!    positives from prose `[1]` / `vec![1]` / Markdown link refs.
 //! 7. Citation validate — every extracted marker must map to a packed
@@ -83,11 +86,12 @@ type PackedContext = (String, Vec<PackedCitation>, usize);
 /// `RefusalReason::LlmStreamAborted`).
 #[derive(Clone, Debug, serde::Serialize)]
 #[serde(tag = "kind", rename_all = "snake_case")]
-// `Final.answer` carries a full `Answer` (~320B) and is the largest
-// variant; `Token` is the hot path. Size mismatch is unavoidable
-// without boxing the wire-shape, which would force every consumer
-// (TUI / CLI / future MCP) to deref. The sink is short-lived (one
-// per ask) so the per-event overhead is not material.
+// p9-fb-33: clippy flags Final.answer (~320B) as the heavy variant.
+// In practice RetrievalDone.hits (Vec<SearchHit>, k≤10×~1KB each)
+// dominates per-emit cost, but it fires once. Boxing either would
+// force every consumer (TUI, CLI ndjson driver, future MCP) to
+// deref through a Box for marginal win on a short-lived per-ask
+// channel. Keep both unboxed.
 #[allow(clippy::large_enum_variant)]
 pub enum StreamEvent {
     RetrievalDone {

crates/kebab-rag/tests/streaming_events.rs

@@ -129,3 +129,89 @@ fn ask_records_llm_stream_aborted_when_receiver_drops() {
     // Persistence still happens on cancel — the row is the audit trail.
     assert_eq!(env.count_answers(), 1, "answers row written on cancel");
 }
+
+/// p9-fb-33 (PR #124 round 1, item 5): pin the "no Final on cancel"
+/// invariant. Uses a barrier-gated LM so the test can observe the
+/// `RetrievalDone` event before any `Token`/`Final` lands in the
+/// channel — then drops `rx` to force SendError on the next `Token`.
+/// The pipeline's cancel branch must avoid emitting `Final` and
+/// record `RefusalReason::LlmStreamAborted`.
+struct BlockingLm {
+    inner: MockLanguageModel,
+    /// Pipeline thread waits on this before yielding any token.
+    /// Test thread releases it after observing `RetrievalDone`.
+    gate: Arc<std::sync::Barrier>,
+}
+
+impl LanguageModel for BlockingLm {
+    fn model_ref(&self) -> kebab_core::ModelRef {
+        self.inner.model_ref()
+    }
+    fn context_tokens(&self) -> usize {
+        self.inner.context_tokens()
+    }
+    fn generate_stream(
+        &self,
+        req: kebab_core::GenerateRequest,
+    ) -> anyhow::Result<Box<dyn Iterator<Item = anyhow::Result<TokenChunk>> + Send>> {
+        // Block until the test signals — guarantees `RetrievalDone`
+        // arrives at the receiver before any `Token` is queued.
+        self.gate.wait();
+        self.inner.generate_stream(req)
+    }
+}
+
+#[test]
+fn ask_emits_no_final_when_cancelled_mid_stream() {
+    use std::sync::Barrier;
+
+    let env = RagEnv::new();
+    let cid = id32("c1");
+    let did = id32("d1");
+    env.seed_chunk(&cid, &did, "notes/a.md", "apples are red.", &["Intro"]);
+    let hits = vec![mk_hit(1, &cid, &did, "notes/a.md", 0.85, &["Intro"])];
+    let retriever: Arc<dyn Retriever> = Arc::new(MockRetriever::new(hits));
+
+    let gate = Arc::new(Barrier::new(2));
+    let lm: Arc<dyn LanguageModel> = Arc::new(BlockingLm {
+        inner: MockLanguageModel {
+            model_id: TEST_LM_ID.to_string(),
+            provider: "mock".to_string(),
+            context_tokens: 32_768,
+            canned_response: "apples are red. [#1]".to_string(),
+            canned_finish: FinishReason::Stop,
+            canned_usage: TokenUsage {
+                prompt_tokens: 10,
+                completion_tokens: 5,
+                latency_ms: 7,
+            },
+        },
+        gate: Arc::clone(&gate),
+    });
+    let pipeline = RagPipeline::new(env.config.clone(), retriever, lm, env.sqlite.clone());
+
+    let (tx, rx) = mpsc::channel::<StreamEvent>();
+    let opts = opts_with_sink(tx);
+    let handle = std::thread::spawn(move || pipeline.ask("apples", opts));
+
+    // Receive RetrievalDone first — pipeline emits this before
+    // calling generate_stream (where the LM blocks on the gate).
+    let first = rx.recv().expect("RetrievalDone must arrive");
+    assert!(
+        matches!(first, StreamEvent::RetrievalDone { .. }),
+        "first event must be RetrievalDone, got {first:?}",
+    );
+
+    // Drop rx now, BEFORE releasing the gate. Once the LM unblocks
+    // and the pipeline tries to send the first Token, it'll get
+    // SendError → cancel branch.
+    drop(rx);
+    gate.wait();
+
+    let ans = handle.join().expect("ask thread").unwrap();
+
+    // Cancel was observed: no Final emitted, refusal recorded.
+    assert!(!ans.grounded);
+    assert_eq!(ans.refusal_reason, Some(RefusalReason::LlmStreamAborted));
+    assert_eq!(env.count_answers(), 1, "answers row written on cancel");
+}