## Code Review: Add `--parallel` flag to `propose-beliefs`

### expert_build/cli.py
VERDICT: PASS
CORRECTNESS: VALID
SPEC_COMPLIANCE: N/A
ISSUE_COMPLIANCE: N/A
BELIEF_COMPLIANCE: N/A
TEST_COVERAGE: COVERED
INTEGRATION: WIRED
REASONING: Standard argparse addition. `type=int, default=1` is correct. Consistent with the existing `--parallel` flag added to `summarize` in commit c99ce99.

---

### expert_build/propose.py:cmd_propose_beliefs
VERDICT: CONCERN
CORRECTNESS: QUESTIONABLE
SPEC_COMPLIANCE: N/A
ISSUE_COMPLIANCE: N/A
BELIEF_COMPLIANCE: N/A
TEST_COVERAGE: PARTIAL
INTEGRATION: WIRED
REASONING: The async conversion is structurally sound — semaphore pattern is correct, `asyncio.gather` preserves result order, read-only shared state (`existing_beliefs`, `belief_vectors`, `batch_paths`) avoids race conditions, and `max(1, ...)` prevents a `parallel=0` deadlock (matching the fix in 3d4e70f). Two concerns:

1. **Dropped user feedback**: The `total_skipped` counter and its `print(f"Filtered {total_skipped} already-accepted beliefs")` message were removed entirely. This is a minor UX regression — users no longer see how many duplicate beliefs were filtered. The filtering logic itself is preserved (the `if bid in existing_ids: continue` loop inside `_process_batch`), but the count is silently lost.

2. **No test for `parallel > 1`**: Every test uses the default `parallel=1`. The core new feature — concurrent LLM calls — is completely untested. A test with `parallel=2` and multiple batches would verify that concurrency actually works and results are correctly aggregated. Without it, a regression in the gather/semaphore path would go undetected.

The `asyncio.run()` call is safe given all current callers (CLI and pipeline) are synchronous, but would break if called from an existing event loop. The pipeline caller (`expert_build/pipeline.py:146`) doesn't set `parallel` on its `SimpleNamespace`, but `getattr(args, "parallel", 1)` handles this gracefully.

---

### expert_build/propose.py:_process_batch
VERDICT: PASS
CORRECTNESS: VALID
SPEC_COMPLIANCE: N/A
ISSUE_COMPLIANCE: N/A
BELIEF_COMPLIANCE: N/A
TEST_COVERAGE: PARTIAL
INTEGRATION: WIRED
REASONING: Clean extraction of the per-batch logic into an async helper. The return type `(filtered, paths) | None` is clear. The retry path (`await invoke(... + RETRY_JSON ...)`) correctly mirrors the original synchronous retry logic. Closure captures are all read-only. The function is well-scoped.

---

### tests/test_propose.py
VERDICT: CONCERN
CORRECTNESS: VALID
SPEC_COMPLIANCE: N/A
ISSUE_COMPLIANCE: N/A
BELIEF_COMPLIANCE: N/A
TEST_COVERAGE: PARTIAL
INTEGRATION: WIRED
REASONING: All existing tests correctly migrated from `patch("expert_build.propose.invoke_sync", ...)` to `patch("expert_build.propose.invoke", new_callable=AsyncMock, ...)`. The `make_args` helper correctly adds `parallel=1`. However, there is no test exercising `parallel > 1`, which is the entire point of this change. At minimum, one test should verify that multiple batches process correctly with `parallel=2` — e.g., asserting that beliefs from all batches appear in the output when run concurrently. The `test_propose_batches_entries` test with `batch_size=2` and 3 entries (2 batches) would be a natural candidate to clone with `parallel=2`.

---

### SELF_REVIEW
LIMITATIONS: Could not run the test suite to verify all tests still pass with the AsyncMock migration. Could not verify whether `_build_dedup_context` or `_score_by_embedding`/`_score_by_keywords` have any hidden mutable state that would be unsafe under concurrency. The pipeline integration (`expert_build/pipeline.py:_stage_extract`) constructs its own `SimpleNamespace` without `parallel` — verified via observation data that `getattr` with default handles this, but could not run the pipeline end-to-end.

---

### FEATURE_REQUESTS
- Include the full test file (not just the diff) when tests are modified, so I can verify assertions still match the modified behavior holistically
- Show the `SimpleNamespace` construction in callers like `pipeline.py:_stage_extract` when a function's signature/expected attributes change

---

**Summary**: The async conversion is correct and well-structured, following the same pattern established in the `summarize` parallelization (c99ce99). Two items worth addressing: (1) add at least one test with `parallel=2` to cover the new feature path, and (2) consider restoring the filtered-duplicates count message or intentionally noting its removal in the commit.
