Merge pull request 'test: make the Ollama connection-refused test deterministic' (#79) from fix/ollama-test-retry-flake into develop

maxRetries: 0 dropped the transport-retry branches below the 79% branch
coverage threshold. Keep the default retries (and assert all 4 attempts) but
stub Math.random to 0 so the backoff is immediate and deterministic.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

.circleci/config.yml

@@ -1,8 +0,0 @@
-version: 2.1
-setup: true
-orbs:
-  munitor: kof22/munitor@dev:snapshot
-workflows:
-  setup:
-    jobs:
-      - munitor/generate_pipeline

.gitea/workflows/build.yaml

@@ -0,0 +1,13 @@
+name: build
+on:
+  push:
+jobs:
+  node:
+    uses: self:KofTwentyTwo/ci-workflows/.gitea/workflows/node.yaml@v1
+    secrets: inherit
+    with:
+      node-version: '22'
+      # check = typecheck + vitest with the coverage thresholds enforced
+      commands: |
+        npm run build
+        npm run check

.munitor.yml

@@ -1,23 +0,0 @@
-# obsidian-plugin pipeline ships only in the Munitor `dev:snapshot` orb
-# build for now; v0.2.x stable releases don't include this template.
-# Bump to a stable tag once Munitor ships obsidian-plugin in a tagged release.
-pipeline: obsidian-plugin
-orb_version: dev:snapshot
-node_version: "20"
-sast: true
-sbom: true
-test:
-  # `npm run check` = typecheck + vitest with coverage thresholds enforced.
-  # Vitest exits non-zero on threshold violation, failing the build (gate 1).
-  commands: ["npm run build", "npm run check"]
-  coverage:
-    tool: vitest
-    command: npm run test:coverage
-    # The orb's node_coverage job parses this summary and enforces
-    # min_instruction independently — second layer of defense in case the
-    # vitest threshold ever gets misconfigured. Ratchet up alongside
-    # vitest.config.ts thresholds.
-    summary_path: coverage/coverage-summary.json
-    min_instruction: "90"
-contexts:
-  github: github

docs/SESSION-STATE.md

@@ -1,126 +0,0 @@
-# Session State
-
-**Last Updated:** 2026-04-29 (overnight + early-morning autonomous run, paused for cross-machine handoff)
-
-## TL;DR for the next session (any machine)
-
-`feature/issue-30-warn-unknown-tags` is open with WIP code that needs parser tests added before it ships. Everything else is clean. PRs #65 (#17), #66 (#15), #67 (#31), #68 (#16), #69 (#5), #70 (#33) all merged to develop. Two backlog items remain in the original "fix them all" pass: **#30 (in progress, WIP committed)** and **#14 maxTokens cap (not started)**.
-
-## What shipped overnight (all merged to develop)
-
-| Issue | PR | Subject |
-|---|---|---|
-| #17 | #65 | SSE error events surfaced across all providers |
-| #15 | #66 | Retry on 429 / transient 5xx with backoff |
-| #31 | #67 | `.state.json.bak` snapshot before overwrite |
-| #16 | #68 | Preserve whitespace in LLM responses |
-| #5  | #69 | Correct dialogue regex; support all quote styles |
-| #33 | #70 | Broaden detectCharacters regex (two-word, hyphen, apostrophe, all-caps names) |
-
-## Active branch: `feature/issue-30-warn-unknown-tags`
-
-Current commit: `1fbbf6f` (marked WIP). Adds `parseAnnotationsWithWarnings()` to `src/parser.ts`, plus `ParserWarning` / `ParsedAnnotations` types, and seeds `flags` in `pipeline.ts` from parser warnings so unknown tags / empty instructions / unclosed multi-line blocks surface in the review note's "Flags for Author" section.
-
-`src/types.ts` extends `ReviewFlag.type` with three new variants:
-- `unknown_tag`
-- `empty_instruction`
-- `unclosed_annotation`
-
-### What's still needed before this can ship
-
-Coverage gates currently fail. Floors are 91 / 90 / 97 / 79 (lines / statements / functions / branches); current measurement is 90.77 / 89.7 / 96.96 / 78.27. Per-file `src/parser.ts` is at lines 79.36% (floor 88) and functions 92.85% (floor 100). The new code paths are untested.
-
-**Add to `test/parser.test.ts`:**
-
-```ts
-import { parseAnnotationsWithWarnings } from "../src/parser";
-
-describe("parseAnnotationsWithWarnings", () => {
-  it("warns on unknown tag", () => {
-    const { annotations, warnings } = parseAnnotationsWithWarnings("%% REWIRTE: foo %%");
-    expect(annotations).toHaveLength(0);
-    expect(warnings).toHaveLength(1);
-    expect(warnings[0]).toMatchObject({ type: "unknown_tag", line: 1 });
-    expect(warnings[0].description).toContain("REWIRTE");
-  });
-
-  it("warns on empty instruction", () => {
-    const { warnings } = parseAnnotationsWithWarnings("%% REWRITE:  %%");
-    expect(warnings).toHaveLength(1);
-    expect(warnings[0]).toMatchObject({ type: "empty_instruction", line: 1 });
-  });
-
-  it("warns on unclosed multi-line annotation", () => {
-    const { warnings } = parseAnnotationsWithWarnings("%% REWRITE: open\nfoo\nbar");
-    expect(warnings).toHaveLength(1);
-    expect(warnings[0]).toMatchObject({ type: "unclosed_annotation", line: 1 });
-  });
-
-  it("returns empty warnings when all annotations are valid", () => {
-    const content = '%% REWRITE: tighten this %%\n%% TONE: lighter %%';
-    const { annotations, warnings } = parseAnnotationsWithWarnings(content);
-    expect(annotations.length).toBeGreaterThan(0);
-    expect(warnings).toHaveLength(0);
-  });
-
-  it("returns valid annotations alongside warnings for invalid ones", () => {
-    const content = '%% REWRITE: keep this %%\n%% NOPE: drop this %%';
-    const { annotations, warnings } = parseAnnotationsWithWarnings(content);
-    expect(annotations).toHaveLength(1);
-    expect(warnings).toHaveLength(1);
-    expect(warnings[0].type).toBe("unknown_tag");
-  });
-});
-```
-
-After tests pass: amend the WIP commit (or new commit), push, open PR.
-
-## Remaining backlog from the "fix them all" pass
-
-- [ ] **#30** Warn on unknown annotation tags — WIP on `feature/issue-30-warn-unknown-tags`, needs the tests above.
-- [ ] **#14** Cap `maxTokens` against per-model output limits — not started. Plan: extend `ModelInfo.maxOutputTokens`, populate for ANTHROPIC_MODELS / GOOGLE_MODELS / OPENAI_MODELS, clamp in `pipeline.ts:299` (`maxTokens: Math.min(settings.maxTokens, modelInfo?.maxOutputTokens ?? settings.maxTokens)`), and log when clamping happens.
-
-## Backlog NOT touched (need user direction)
-
-These need UX or architectural decisions:
-
-- **#46** Rollback command — UX choice: do we delete intermediate version files, or just rewind `.version`?
-- **#9** Parallel annotations — concurrency tradeoffs: shared rate-limit bucket vs. per-provider concurrency cap. Now unblocked by #15 retry.
-- **#13** Dry-run cost preview — modal UX choice.
-- **#7** `commands.ts` refactor — large architectural change; defer until time permits.
-- **47 parked items in #62** — community-store gating, CI hygiene, forward-leaning features.
-
-## Tier overview after this run
-
-| Tier | Done | Open |
-|---|---|---|
-| Tier 1 (block real writing) | #18, #17, #15 | #46 rollback |
-| Tier 2 (cost & speed)       | #16            | #13, #9, #12, #14 |
-| Tier 3 (correctness drift)  | #5, #31, #33   | **#30 (WIP)**, #14 |
-
-## Resume instructions on another machine
-
-```bash
-cd ~/Git.Local/kof22/obsidian-penny  # or wherever you keep it
-git fetch origin
-git checkout feature/issue-30-warn-unknown-tags
-git pull
-npm install   # in case lockfile bumped on the other machine
-npm run check # confirm the coverage failure described above
-```
-
-Then add the tests in `test/parser.test.ts` per the snippet above, run `npm run check`, amend `1fbbf6f` (or add a new commit), push, open PR.
-
-## Coverage thresholds (current floors on develop)
-
-- `vitest.config.ts`: lines 91 / statements 90 / functions 97 / branches 79
-- `.munitor.yml`: `min_instruction: 90`
-- Per-file ratchets locked in vitest.config.ts: `node-stream.ts` lines 96, etc.
-
-## Key Reference
-
-- **Audit tracker (master):** https://github.com/KofTwentyTwo/obsidian-penny/issues/62
-- **Recent PRs:** #65–#70 all merged to develop
-- **Open PRs:** none currently. PR #66's session-state file may still be uncommitted on the prior machine.
-- **Local dev loop:** `cd /Users/james.maes/Git.Local/kof22/obsidian-penny && npm run dev` — Hot Reload picks up bundle changes automatically
-- **Books vault plugin path:** `/Users/james.maes/Git.Local/kof22/Books/.obsidian/plugins/penny/` (three files are symlinks → repo root)

docs/TODO.md

@@ -1,38 +0,0 @@
-# TODO
-
-Personal-use sprint — issues that block daily writing use of PENNY.
-Full audit list and tracker: [#62](https://github.com/KofTwentyTwo/obsidian-penny/issues/62).
-
-## Tier 1 — Will bite during real sessions
-
-- [x] **#18** LLM call timeout (PR #64 merged)
-- [x] **#17** SSE error events surfaced (PR #65, ready to merge)
-- [ ] **#15** Retry on 429 / transient 5xx with `Retry-After` + exponential backoff
-- [ ] **#46** Roll-back command (revert to a previous chapter version cleanly)
-
-## Tier 2 — Cost & speed quality of life
-
-- [ ] **#13** Dry-run cost preview + confirmation gate above threshold
-- [ ] **#9** Process annotations in parallel (configurable concurrency) — *depends on #15 retry*
-- [ ] **#12** Multi-line annotation cap surfaces warning instead of silent truncate
-- [ ] **#16** Remove provider `.trim()` — preserve meaningful whitespace
-
-## Tier 3 — Correctness drift
-
-- [ ] **#5** Fix dialogue regex (single-quoted/British dialogue currently uncounted)
-- [ ] **#30** Warn on unknown annotation tags (typo'd `%% REWIRTE: ... %%` is silently dropped)
-- [ ] **#33** `detectCharacters` regex misses two-word names, apostrophes, all-caps shouted dialogue
-- [ ] **#31** Write `.bak` snapshot of `.state.json` before overwrite (recovery)
-- [ ] **#14** Cap `maxTokens` against per-model output limits (avoid opaque 400s)
-
-## Out of scope for this sprint
-
-The remaining 47 audit-derived issues are tracked in [#62](https://github.com/KofTwentyTwo/obsidian-penny/issues/62). They cover community-store gating (#4 quotes legal, #8 inline styles, #19 API key warnings, #26 mobile guard), the big `commands.ts` refactor (#7), CI hygiene (#40, #43), and forward-leaning features. Pick up after this sprint stabilizes.
-
-## Workflow notes
-
-- Branch from `develop` as `feature/issue-XX-short-name`.
-- TDD: write a failing test first, prove it fails on `develop`, then fix.
-- Each PR ratchets `vitest.config.ts` per-file thresholds up to lock in coverage gains.
-- `npm run check` mirrors CI: typecheck + tests + coverage gate.
-- Hot Reload watches `Books/.obsidian/plugins/penny/main.js` (symlinked to repo root); `npm run dev` keeps the bundle fresh on save.

src/parser.ts

@@ -21,24 +21,10 @@
 import {
   type AnnotatedSection,
   type AnnotationTag,
-  type ReviewFlag,
   ACTIONABLE_TAGS,
   ALL_TAGS,
 } from "./types";
 
-/**
- * Parser warnings raised during `parseAnnotationsWithWarnings`. Surfaced in
- * the review note's "Flags for Author" section so authors notice typo'd tags
- * (e.g. `REWIRTE`) and unclosed multi-line annotations that would otherwise
- * be silently dropped (issue #30).
- */
-export type ParserWarning = ReviewFlag;
-
-export interface ParsedAnnotations {
-  annotations: AnnotatedSection[];
-  warnings: ParserWarning[];
-}
-
 /** Single-line annotation: `%% TAG: instruction %%` on one line. Captures tag and instruction. */
 const ANNOTATION_PATTERN = /%%\s*([A-Z]+)\s*:\s*(.*?)\s*%%/;
 
@@ -218,19 +204,8 @@ function resolveScope(
  *          silently skipped.
  */
 export function parseAnnotations(content: string): AnnotatedSection[] {
-  return parseAnnotationsWithWarnings(content).annotations;
-}
-
-/**
- * Same parse as `parseAnnotations`, but also returns warnings about malformed
- * annotations the parser dropped: unknown tags (typos), empty instructions,
- * and unclosed multi-line blocks. Surface these in the review note so the
- * author sees them instead of silently losing the annotation (issue #30).
- */
-export function parseAnnotationsWithWarnings(content: string): ParsedAnnotations {
   const lines = content.split("\n");
   const results: AnnotatedSection[] = [];
-  const warnings: ParserWarning[] = [];
 
   for (let i = 0; i < lines.length; i++) {
     const line = lines[i];
@@ -244,21 +219,8 @@ export function parseAnnotationsWithWarnings(content: string): ParsedAnnotations
       const tag = match[1];
       const instruction = match[2].trim();
 
-      // Surface malformed annotations as warnings instead of silently dropping.
-      if (!isValidTag(tag)) {
-        warnings.push({
-          type: "unknown_tag",
-          line: i + 1,
-          description: `Unknown annotation tag "${tag}" -- did you mean one of: ${ALL_TAGS.slice(0, 6).join(", ")}, ...?`,
-        });
-        continue;
-      }
-      if (instruction.length === 0) {
-        warnings.push({
-          type: "empty_instruction",
-          line: i + 1,
-          description: `Empty instruction for tag ${tag}; nothing to process`,
-        });
+      // Skip malformed: unrecognised tag or empty instruction.
+      if (!isValidTag(tag) || instruction.length === 0) {
         continue;
       }
 
@@ -321,29 +283,6 @@ export function parseAnnotationsWithWarnings(content: string): ParsedAnnotations
         }
 
         const instruction = instructionParts.join(" ").trim();
-        // Detect unclosed multi-line annotations: closeLine is still i when
-        // no closing %% was found within the 20-line scan window.
-        if (closeLine === i) {
-          warnings.push({
-            type: "unclosed_annotation",
-            line: i + 1,
-            description: `Unclosed multi-line annotation opened with %% ${tag}: -- missing closing %% within 20 lines`,
-          });
-          continue;
-        }
-        if (!isValidTag(tag)) {
-          warnings.push({
-            type: "unknown_tag",
-            line: i + 1,
-            description: `Unknown annotation tag "${tag}" -- did you mean one of: ${ALL_TAGS.slice(0, 6).join(", ")}, ...?`,
-          });
-        } else if (instruction.length === 0) {
-          warnings.push({
-            type: "empty_instruction",
-            line: i + 1,
-            description: `Empty instruction for tag ${tag}; nothing to process`,
-          });
-        }
         if (isValidTag(tag) && instruction.length > 0) {
           // Use the line AFTER the closing %% as the annotation target
           const annotationLineIndex = closeLine;
@@ -386,7 +325,5 @@ export function parseAnnotationsWithWarnings(content: string): ParsedAnnotations
 
   // Sort by lineStart ascending (stable).
   results.sort((a, b) => a.lineStart - b.lineStart);
-  // Sort warnings by line so the review note presents them top-down.
-  warnings.sort((a, b) => a.line - b.line);
-  return { annotations: results, warnings };
+  return results;
 }

src/pipeline.ts

@@ -25,7 +25,7 @@ import type { ContextFiles } from "./context";
 import type { CompletionRequest, CompletionResponse } from "./providers/service";
 import type { RouteConfig, ComplexityTier } from "./providers/router";
 import { TAG_COMPLEXITY } from "./providers/router";
-import { parseAnnotations, parseAnnotationsWithWarnings } from "./parser";
+import { parseAnnotations } from "./parser";
 import { assembleContext } from "./context";
 import { buildPrompt, callProvider } from "./drafter";
 import { assembleNewVersion } from "./assembler";
@@ -176,12 +176,8 @@ export async function runPipeline(input: PipelineInput): Promise<PipelineResult
   const { content, settings, chapterId, bookId } = input;
   const onProgress = input.onProgress;
 
-  // (a) Parse annotations (use pre-parsed if provided to avoid redundant work).
-  // Always also collect parser warnings (unknown tags, empty instructions,
-  // unclosed multi-line annotations) so they surface in the review note.
-  const parsed = parseAnnotationsWithWarnings(content);
-  const allAnnotations = input.preParsedAnnotations ?? parsed.annotations;
-  const parserWarnings = parsed.warnings;
+  // (a) Parse annotations (use pre-parsed if provided to avoid redundant work)
+  const allAnnotations = input.preParsedAnnotations ?? parseAnnotations(content);
 
   // (b) Read version and state
   const currentVersion = readVersion(input.versionContent);
@@ -213,10 +209,7 @@ export async function runPipeline(input: PipelineInput): Promise<PipelineResult
   // (e) Process each annotation via the LLM provider
   const revisions: Array<{ annotation: AnnotatedSection; revisedText: string }> = [];
   const changes: AnnotationChange[] = [];
-  // Seed the review flags with any parser warnings (issue #30) so authors
-  // see typo'd tags / empty instructions / unclosed blocks they would have
-  // otherwise lost silently.
-  const flags: ReviewFlag[] = [...parserWarnings];
+  const flags: ReviewFlag[] = [];
   const processedTags: string[] = [];
   let hadErrors = false;
   let completedCount = 0;

src/types.ts

@@ -507,14 +507,7 @@ export interface AnnotationChange {
  */
 export interface ReviewFlag {
   /** Category of the flag. */
-  type:
-    | "research"
-    | "plot"
-    | "vague"
-    | "error"
-    | "unknown_tag"
-    | "empty_instruction"
-    | "unclosed_annotation";
+  type: "research" | "plot" | "vague" | "error";
   /** Line number associated with the flag. */
   line: number;
   /** Human-readable description of what needs attention. */

test/providers/ollama.test.ts

@@ -1,4 +1,4 @@
-import { describe, it, expect } from "vitest";
+import { describe, it, expect, vi } from "vitest";
 import { OllamaProvider } from "../../src/providers/ollama";
 import type { HttpFn, HttpResponse, CompletionRequest } from "../../src/providers/service";
 
@@ -283,14 +283,24 @@ describe("OllamaProvider", () => {
     });
 
     it("throws connection refused message when Ollama is not running", async () => {
+      let attempts = 0;
       const fn: HttpFn = async () => {
+        attempts++;
         throw new Error("ECONNREFUSED");
       };
       const provider = new OllamaProvider(fn);
+      // Zero the retry jitter: the default 3 retries back off up to 1+2+4 s at random,
+      // which intermittently exceeded the 5 s test timeout.
+      const random = vi.spyOn(Math, "random").mockReturnValue(0);
 
-      await expect(
-        provider.complete(makeRequest()),
-      ).rejects.toThrow("Cannot connect to Ollama at http://localhost:11434. Is Ollama running?");
+      try {
+        await expect(
+          provider.complete(makeRequest()),
+        ).rejects.toThrow("Cannot connect to Ollama at http://localhost:11434. Is Ollama running?");
+        expect(attempts).toBe(4);
+      } finally {
+        random.mockRestore();
+      }
     });
   });