Add CLI search --needs-producer-review parsing tests

docs/github-throttling.md

@@ -0,0 +1,84 @@
+# GitHub API throttling configuration
+
+This document describes environment variables that control the shared GitHub API throttler used by Worklog.
+
+Location
+- Implementation: src/github-throttler.ts
+- Default shared instance: exported as `throttler` from src/github-throttler.ts
+
+Environment variables
+
+- WL_GITHUB_RATE
+  - Meaning: Token refill rate (tokens per second). Each API call consumes one token.
+  - Default: 2
+  - Effect: Controls sustained request rate. Higher values allow more continuous throughput.
+  - Example: `WL_GITHUB_RATE=5` allows an average of 5 requests per second.
+
+- WL_GITHUB_BURST
+  - Meaning: Bucket capacity (maximum number of tokens accumulated).
+  - Default: 4
+  - Effect: Allows short bursts above WL_GITHUB_RATE up to the burst size. A larger burst lets the system absorb short spikes in activity.
+  - Example: `WL_GITHUB_BURST=10` permits short bursts of up to 10 immediate requests if tokens are available.
+
+- WL_GITHUB_CONCURRENCY
+  - Meaning: Global concurrency cap for concurrent GitHub API requests enforced by the central throttler.
+  - Default: unset (no concurrency cap). When unset the throttler only enforces rate limits.
+  - Notes: If you explicitly set the value to `0` or a negative number the throttler treats this as "unlimited" (Infinity). To disable the concurrency cap leave the variable unset.
+  - Effect: When set to a positive integer the throttler will never run more than that many requests concurrently across the process.
+  - Example: `WL_GITHUB_CONCURRENCY=4` limits concurrent GitHub API calls to 4.
+
+- WL_GITHUB_THROTTLER_DEBUG
+  - Meaning: Enable debug logging in the throttler implementation.
+  - Default: unset/false
+  - Example: `WL_GITHUB_THROTTLER_DEBUG=true`
+
+Tuning guidance
+
+- Start with conservative defaults
+  - Defaults (rate=2, burst=4) are safe for small teams and avoid triggering GitHub abuse or secondary rate limits. Try increasing gradually.
+
+- Increase WL_GITHUB_RATE for sustained throughput
+  - If your usage is CPU/network-bound and GitHub's API rate limits permit it, increase `WL_GITHUB_RATE` to allow more sustained requests per second.
+  - Keep `WL_GITHUB_BURST` modestly larger than `WL_GITHUB_RATE` so short spikes are absorbed without large bursts that could provoke abuse detection.
+
+- Use WL_GITHUB_CONCURRENCY to limit parallelism
+  - If the process launches many concurrent requests (e.g. large sync operations) set `WL_GITHUB_CONCURRENCY` to a value matching your acceptable parallelism (network capacity, token pool size, or pacing requirements).
+  - Setting a concurrency cap can be safer than relying only on rate limiting if individual requests are slow or block other resources.
+
+- Watch for GitHub rate limits and abuse responses
+  - GitHub may throttle or return secondary rate-limit responses. If you see increased 403/429 or abuse-related responses, reduce rate/concurrency and increase monitoring. Use `WL_GITHUB_THROTTLER_DEBUG` during testing to inspect throttler behavior.
+
+Migration notes
+
+- Behaviour change when enabling WL_GITHUB_CONCURRENCY
+  - Historically the code relied on local per-callsite concurrency controls. Once you set `WL_GITHUB_CONCURRENCY` a single global concurrency cap is enforced by the central throttler. Verify sync and CI workloads after enabling to ensure you don't unintentionally over-constrain throughput.
+
+- Where to look in code
+  - See src/github-throttler.ts for implementation details (TokenBucketThrottler and makeThrottlerFromEnv). The defaults are defined in that file:
+    - rate default: 2
+    - burst default: 4
+    - concurrency default: Infinity (when WL_GITHUB_CONCURRENCY is unset)
+
+- Recommended rollout
+  1. Test changes in a development environment with `WL_GITHUB_THROTTLER_DEBUG=true`.
+  2. Increase `WL_GITHUB_RATE` gradually and monitor API responses and CI job durations.
+  3. If enabling `WL_GITHUB_CONCURRENCY` start with a permissive value (e.g. 10) and lower it if resource contention is observed.
+
+Examples
+
+- Conservative: use defaults (no changes)
+  - no env vars set
+
+- Moderate throughput with limited bursts and concurrency
+  - WL_GITHUB_RATE=5
+  - WL_GITHUB_BURST=8
+  - WL_GITHUB_CONCURRENCY=6
+
+- Debugging
+  - WL_GITHUB_THROTTLER_DEBUG=true
+
+Acceptance criteria
+
+- This file documents WL_GITHUB_CONCURRENCY, WL_GITHUB_RATE and WL_GITHUB_BURST, lists defaults and provides tuning guidance, references src/github-throttler.ts, and provides migration notes.
+
+If you'd like this added to the top-level README instead, or to a different docs page, tell me and I will move it there.

tests/cli/issue-status.test.ts

@@ -129,6 +129,34 @@ describe('CLI Issue Status Tests', () => {
       result.workItems.forEach((item: any) => expect(item.needsProducerReview).not.toBe(true));
     });
 
+    it('should accept "yes" as true for needs-producer-review', async () => {
+      const { stdout } = await execAsync(`tsx ${cliPath} --json list --needs-producer-review yes`);
+
+      const result = JSON.parse(stdout);
+      expect(result.success).toBe(true);
+      expect(result.workItems).toHaveLength(2);
+      result.workItems.forEach((item: any) => expect(item.needsProducerReview).toBe(true));
+    });
+
+    it('should accept "no" as false for needs-producer-review', async () => {
+      const { stdout } = await execAsync(`tsx ${cliPath} --json list --needs-producer-review no`);
+
+      const result = JSON.parse(stdout);
+      expect(result.success).toBe(true);
+      expect(result.workItems).toHaveLength(1);
+      result.workItems.forEach((item: any) => expect(item.needsProducerReview).not.toBe(true));
+    });
+
+    it('should error for invalid needs-producer-review value', async () => {
+      try {
+        await execAsync(`tsx ${cliPath} --json list --needs-producer-review maybe`);
+        expect.fail('Should have thrown an error');
+      } catch (error: any) {
+        const result = JSON.parse(error.stderr || '{}');
+        expect(result.success).toBe(false);
+      }
+    });
+
     it('should include completed items when --stage filter matches them', async () => {
       // Seed items where a completed item has a specific stage
       seedWorkItems(tempState.tempDir, [
@@ -531,4 +559,87 @@ describe('CLI Issue Status Tests', () => {
       expect(stdout).toContain('search');
     });
   });
+
+  describe('search --needs-producer-review parsing', () => {
+    let reviewItem1Id: string;
+    let reviewItem2Id: string;
+    let nonReviewItemId: string;
+
+    beforeEach(async () => {
+      // Create items via CLI so they're in the SQLite database for search
+      const r1 = JSON.parse((await execAsync(`tsx ${cliPath} --json create -t "Review item 1" -p high`)).stdout);
+      const r2 = JSON.parse((await execAsync(`tsx ${cliPath} --json create -t "Review item 2" -p medium`)).stdout);
+      const r3 = JSON.parse((await execAsync(`tsx ${cliPath} --json create -t "Non-review item" -p low`)).stdout);
+
+      reviewItem1Id = r1.workItem.id;
+      reviewItem2Id = r2.workItem.id;
+      nonReviewItemId = r3.workItem.id;
+
+      // Set needsProducerReview flags
+      await execAsync(`tsx ${cliPath} --json update ${reviewItem1Id} --needs-producer-review true`);
+      await execAsync(`tsx ${cliPath} --json update ${reviewItem2Id} --needs-producer-review true`);
+      await execAsync(`tsx ${cliPath} --json update ${nonReviewItemId} --needs-producer-review false`);
+    });
+
+    it('should filter search results by --needs-producer-review true', async () => {
+      const { stdout } = await execAsync(`tsx ${cliPath} --json search "item" --needs-producer-review true`);
+
+      const result = JSON.parse(stdout);
+      expect(result.success).toBe(true);
+      expect(result.results).toHaveLength(2);
+      const ids = result.results.map((r: any) => r.id);
+      expect(ids).toContain(reviewItem1Id);
+      expect(ids).toContain(reviewItem2Id);
+    });
+
+    it('should default --needs-producer-review to true when value omitted', async () => {
+      const { stdout } = await execAsync(`tsx ${cliPath} --json search "item" --needs-producer-review`);
+
+      const result = JSON.parse(stdout);
+      expect(result.success).toBe(true);
+      expect(result.results).toHaveLength(2);
+      const ids = result.results.map((r: any) => r.id);
+      expect(ids).toContain(reviewItem1Id);
+      expect(ids).toContain(reviewItem2Id);
+    });
+
+    it('should filter search results by --needs-producer-review false', async () => {
+      const { stdout } = await execAsync(`tsx ${cliPath} --json search "item" --needs-producer-review false`);
+
+      const result = JSON.parse(stdout);
+      expect(result.success).toBe(true);
+      expect(result.results).toHaveLength(1);
+      expect(result.results[0].id).toBe(nonReviewItemId);
+    });
+
+    it('should accept "yes" as true for --needs-producer-review', async () => {
+      const { stdout } = await execAsync(`tsx ${cliPath} --json search "item" --needs-producer-review yes`);
+
+      const result = JSON.parse(stdout);
+      expect(result.success).toBe(true);
+      expect(result.results).toHaveLength(2);
+      const ids = result.results.map((r: any) => r.id);
+      expect(ids).toContain(reviewItem1Id);
+      expect(ids).toContain(reviewItem2Id);
+    });
+
+    it('should accept "no" as false for --needs-producer-review', async () => {
+      const { stdout } = await execAsync(`tsx ${cliPath} --json search "item" --needs-producer-review no`);
+
+      const result = JSON.parse(stdout);
+      expect(result.success).toBe(true);
+      expect(result.results).toHaveLength(1);
+      expect(result.results[0].id).toBe(nonReviewItemId);
+    });
+
+    it('should error for invalid --needs-producer-review value', async () => {
+      try {
+        await execAsync(`tsx ${cliPath} --json search "item" --needs-producer-review maybe`);
+        expect.fail('Should have thrown an error');
+      } catch (error: any) {
+        const result = JSON.parse(error.stderr || '{}');
+        expect(result.success).toBe(false);
+      }
+    });
+  });
 });

tests/fts-search.test.ts

@@ -271,134 +271,6 @@ describe('FTS Search', () => {
     });
   });
 
-  describe('searchFallback with new filter flags', () => {
-    // Test the fallback search path directly via SqlitePersistentStore.searchFallback().
-    // better-sqlite3 always includes FTS5, so we cannot disable it at the
-    // WorklogDatabase level; calling searchFallback() on the store exercises
-    // the application-level filtering code path that would run when FTS5 is
-    // unavailable.
-
-    describe('--priority filter (fallback)', () => {
-      it('should filter by priority', () => {
-        db.create({ title: 'Fbpriority alpha task', priority: 'high' });
-        db.create({ title: 'Fbpriority alpha chore', priority: 'low' });
-
-        const results = (db as any).store.searchFallback('fbpriority alpha', { priority: 'high' });
-        expect(results.length).toBe(1);
-        const item = db.get(results[0].itemId);
-        expect(item?.priority).toBe('high');
-      });
-    });
-
-    describe('--assignee filter (fallback)', () => {
-      it('should filter by assignee', () => {
-        db.create({ title: 'Fbassignee alpha work', assignee: 'alice' });
-        db.create({ title: 'Fbassignee alpha work', assignee: 'bob' });
-
-        const results = (db as any).store.searchFallback('fbassignee alpha', { assignee: 'alice' });
-        expect(results.length).toBe(1);
-        const item = db.get(results[0].itemId);
-        expect(item?.assignee).toBe('alice');
-      });
-    });
-
-    describe('--stage filter (fallback)', () => {
-      it('should filter by stage', () => {
-        db.create({ title: 'Fbstage alpha item', stage: 'review' });
-        db.create({ title: 'Fbstage alpha item', stage: 'done' });
-
-        const results = (db as any).store.searchFallback('fbstage alpha', { stage: 'review' });
-        expect(results.length).toBe(1);
-        const item = db.get(results[0].itemId);
-        expect(item?.stage).toBe('review');
-      });
-    });
-
-    describe('--issue-type filter (fallback)', () => {
-      it('should filter by issueType', () => {
-        db.create({ title: 'Fbtype alpha entry', issueType: 'epic' });
-        db.create({ title: 'Fbtype alpha entry', issueType: 'task' });
-
-        const results = (db as any).store.searchFallback('fbtype alpha', { issueType: 'epic' });
-        expect(results.length).toBe(1);
-        const item = db.get(results[0].itemId);
-        expect(item?.issueType).toBe('epic');
-      });
-    });
-
-    describe('--needs-producer-review filter (fallback)', () => {
-      it('should filter by needsProducerReview true', () => {
-        db.create({ title: 'Fbreview alpha item', needsProducerReview: true });
-        db.create({ title: 'Fbreview alpha item', needsProducerReview: false });
-
-        const results = (db as any).store.searchFallback('fbreview alpha', { needsProducerReview: true });
-        expect(results.length).toBe(1);
-        const item = db.get(results[0].itemId);
-        expect(item?.needsProducerReview).toBe(true);
-      });
-
-      it('should filter by needsProducerReview false', () => {
-        db.create({ title: 'Fbreview beta item', needsProducerReview: true });
-        db.create({ title: 'Fbreview beta item', needsProducerReview: false });
-
-        const results = (db as any).store.searchFallback('fbreview beta', { needsProducerReview: false });
-        expect(results.length).toBe(1);
-        const item = db.get(results[0].itemId);
-        expect(item?.needsProducerReview).toBe(false);
-      });
-    });
-
-    describe('--deleted filter (fallback)', () => {
-      it('should exclude deleted items by default', () => {
-        db.create({ title: 'Fbdeleted alpha item', status: 'open' });
-        // Create an item with status 'deleted' directly (avoids db.delete
-        // which would also remove the FTS entry, allowing us to verify the
-        // fallback filter independently).
-        db.create({ title: 'Fbdeleted alpha item', status: 'deleted' as any });
-
-        const results = (db as any).store.searchFallback('fbdeleted alpha');
-        expect(results.length).toBe(1);
-        const item = db.get(results[0].itemId);
-        expect(item?.status).toBe('open');
-      });
-
-      it('should include deleted items when deleted flag is set', () => {
-        db.create({ title: 'Fbdeleted beta item', status: 'open' });
-        db.create({ title: 'Fbdeleted beta item', status: 'deleted' as any });
-
-        const results = (db as any).store.searchFallback('fbdeleted beta', { deleted: true });
-        expect(results.length).toBe(2);
-      });
-    });
-
-    describe('combined filters (fallback)', () => {
-      it('should combine priority and assignee', () => {
-        db.create({ title: 'Fbcombo alpha work', priority: 'high', assignee: 'alice' });
-        db.create({ title: 'Fbcombo alpha work', priority: 'high', assignee: 'bob' });
-        db.create({ title: 'Fbcombo alpha work', priority: 'low', assignee: 'alice' });
-
-        const results = (db as any).store.searchFallback('fbcombo alpha', { priority: 'high', assignee: 'alice' });
-        expect(results.length).toBe(1);
-        const item = db.get(results[0].itemId);
-        expect(item?.priority).toBe('high');
-        expect(item?.assignee).toBe('alice');
-      });
-
-      it('should combine stage, issueType and needsProducerReview', () => {
-        db.create({ title: 'Fbmulti alpha item', stage: 'review', issueType: 'bug', needsProducerReview: true });
-        db.create({ title: 'Fbmulti alpha item', stage: 'review', issueType: 'bug', needsProducerReview: false });
-        db.create({ title: 'Fbmulti alpha item', stage: 'done', issueType: 'bug', needsProducerReview: true });
-
-        const results = (db as any).store.searchFallback('fbmulti alpha', { stage: 'review', issueType: 'bug', needsProducerReview: true });
-        expect(results.length).toBe(1);
-        const item = db.get(results[0].itemId);
-        expect(item?.stage).toBe('review');
-        expect(item?.issueType).toBe('bug');
-        expect(item?.needsProducerReview).toBe(true);
-      });
-    });
-  });
-
   describe('index updates on write', () => {
     it('should reflect updates in search results', () => {
       const item = db.create({ title: 'Original title alpha' });