ProfSynapse · ProfSynapse · Apr 7, 2026 · Apr 7, 2026
diff --git a/docs/review/midway65-pr-audit-2026-04-07.md b/docs/review/midway65-pr-audit-2026-04-07.md
@@ -0,0 +1,166 @@
+# Midway65 PR Audit — 2026-04-07
+
+Audit of **PR #104** and **PR #105** from **Midway65**, both targeting `main`.
+
+---
+
+## Relationship Between the Two PRs
+
+PR #105 = PR #104 + 3 additional commits. Both PRs share the same massive base: **the entire PR #97 payload** (previously audited, partially rejected). PR #105 is a strict superset.
+
+| PR | Commits | Files | Insertions | Deletions |
+|----|---------|-------|------------|-----------|
+| **#104** | 63 | 40 | +2,248 | -1,890 |
+| **#105** | 66 | 40 | +2,250 | -1,896 |
+| **Delta (#105 - #104)** | 3 | 3 | +4 | -8 |
+
+**Critical framing**: Both PRs are described as focused workspace optimization features, but the actual diff is a 40-file, 66-commit payload that is effectively **PR #97 resubmitted** with workspace changes stacked on top.
+
+---
+
+## PR #97 Baggage Still Present
+
+The following items from the previous PR #97 audit remain in both PRs:
+
+### 1. Schema Migrations v12-v19 — STILL PRESENT (REJECT)
+
+`CURRENT_SCHEMA_VERSION` bumped from 11 to 19. Eight migrations added:
+- **v12**: Empty stub for vec0 dimension fix
+- **v13-v16**: Empty stubs referencing Midway65's local Windows paths (`C:\Users\middl\...`)
+- **v17**: `DROP TABLE IF EXISTS embedding_config`
+- **v18**: Drop/recreate `embedding_metadata` without legacy `dimension` column
+- **v19**: `DROP TABLE IF EXISTS semantic_feedback, block_embedding_metadata`
+
+Plus `SQLiteCacheManager.fixVec0TableDimensions()` that drops/recreates vec0 tables if they have 768-dim from Midway65's abandoned Nomic fork.
+
+**Verdict**: Fork-specific cleanup. Burns 8 schema version numbers for no benefit to upstream users. **Must not merge.**
+
+### 2. Action Bar Feature (Insert/Append/Create File) — STILL PRESENT (REJECT)
+
+New files: `MessageActionBar.ts` (+139), `CreateFileModal.ts` (+123). Previously rejected as UI noise in the PR #97 review. Still undisclosed in the PR description.
+
+### 3. Orphaned JSONL Pruning — STILL PRESENT (REJECT)
+
+`HybridStorageAdapter.pruneOrphanedConversationFiles()` runs at startup, lists all JSONL files, checks each against SQLite, and **deletes** any not found in SQLite. This inverts the JSONL-as-source-of-truth architecture. If SQLite is corrupt or incomplete, legitimate conversation files will be permanently deleted.
+
+Supporting infrastructure added to `JSONLWriter.ts`: `listFiles()`, `deleteFile()`, streaming readline for large files.
+
+### 4. Perplexity Code — CLEAN
+
+Perplexity features were added then fully reverted. No perplexity-specific code remains. Good.
+
+### 5. ProviderHttpClient Fix — STILL PRESENT (ACCEPT separately)
+
+Switches from `await import('node:https')` to `require('https')`, adds `resRef?.destroy(err)` for timeout handling. This is a real fix but undisclosed in PR scope.
+
+### 6. Whitespace Noise — STILL PRESENT
+
+~1,543 lines of CRLF-to-LF conversions across 10 files inflate the diff significantly:
+- `ConversationRepository.ts`, `WorkspaceService.ts`, `ChatService.ts`, `StreamingResponseService.ts`, `StreamingOrchestrator.ts`, `ProvidersTab.ts`, `ChatInput.ts`, `ConversationList.ts`, `MessageDisplay.ts`, `ConversationManager.ts`
+
+---
+
+## New Content: Workspace Optimization (PR #104)
+
+Five new commits implement a "two-tier workspace prompt" system. This is the genuine new feature.
+
+### G-W1 + G-W4: Cheap Restore + Remove Dead Per-Turn Fetches — APPROVE
+
+**What**: Replaces `loadWorkspace()` agent tool execution (which fetches file trees, sessions, states) with a simple `getWorkspaceBasic()` DB lookup during conversation switch/restore. Also removes ~100 lines of per-turn fetch code (`getVaultStructure()`, `listAvailableWorkspaces()`, `getAvailablePromptSummaries()`, `getToolAgentInfo()`) that was **never consumed** by `SystemPromptBuilder.build()`.
+
+**Assessment**: Correct. The old code ran a full agent tool execution on every conversation switch — wasteful and fragile. The dead code removal is verified genuinely dead.
+
+**Minor concern**: On error, `selectedWorkspaceId` is kept but `selectedWorkspaceSlimData` is null, creating a degraded state where the UI shows a workspace but the system prompt has no workspace context. Acceptable tradeoff.
+
+### G-W2: Slim Workspace Header — APPROVE
+
+**What**: Replaces full `JSON.stringify(workspaceData, null, 2)` in the system prompt (~500-2000 tokens) with a ~50-100 token header containing only `id`, `name`, `description`, `purpose`, `rootFolder`, plus a hint to call `memoryManager.loadWorkspace` for details.
+
+**Assessment**: Good design. Real token savings on every turn. The `<active_workspace>` tag is well-structured.
+
+**Minor issue**: Dead fields remain in `SystemPromptOptions` interface (`workspaceContext`, `vaultStructure`, `availableWorkspaces`, `availablePrompts`, `toolAgents`) — never consumed. Harmless but untidy.
+
+### G-W3: First-Message Full Context Load — APPROVE
+
+**What**: Introduces a `pendingFullWorkspaceLoad` flag. On workspace selection, flag is set `true`. On the first message send, the full workspace data is loaded and included in the system prompt. On subsequent turns, only the slim header is used. The flag is consumed (set `false`) in `getModelSettings()`.
+
+**Assessment**: Sound mechanism. The flag lifecycle is clean:
+1. Set `true` in `restoreWorkspace()` / `setWorkspaceContext()` after successful slim fetch
+2. Checked and consumed in `getModelSettings()` (pre-send)
+3. `loadedWorkspaceData` is nulled at the top of the next `getModelSettings()` call
+4. Subagents can still access the data during the same turn (before next `getModelSettings()`)
+
+### G-W3 Regression Fix (e15d1f31) — APPROVE
+
+Preserves `loadedWorkspaceData` after `getModelSettings()` returns so `SubagentController.buildSubagentContext()` can access it during the same message-processing cycle. Correct.
+
+### Code Review Corrections (69c18546) — APPROVE
+
+Adds `String()` wrappers around `workspaceName` and `workspaceId` in `escapeXmlAttribute()` calls. Defensive coding against non-string values from the index signature. Minor but correct.
+
+---
+
+## New Content: Bug Fixes (PR #105 only)
+
+Three additional commits on top of PR #104.
+
+### Bug 1: G-W3 Flag Consumed During Init (6bd34523) — APPROVE
+
+**Problem**: During `ChatView` initialization, `await this.modelAgentManager.getMessageOptions()` is called just to extract `provider` for `lifecycleManager.handleChatViewOpened()`. But `getMessageOptions()` has a side effect: it checks and consumes `pendingFullWorkspaceLoad`, setting it to `false`. This means the G-W3 full-context load fires during init instead of on the user's first message.
+
+**Fix**: Replaces `(await this.modelAgentManager.getMessageOptions()).provider` with `this.modelAgentManager.getSelectedModel()?.providerId` — a synchronous accessor that returns the same provider ID without side effects.
+
+**Assessment**: Real bug. Clean fix. The two accessors are semantically equivalent for this use case.
+
+### Bug 2: Drop Redundant Context Param (e7dcb5d8) — APPROVE
+
+**Problem**: `setWorkspaceContext(workspaceId, context)` takes a `context` parameter that is immediately overwritten by the method's own `getWorkspaceBasic()` call.
+
+**Fix**: Removes the `context` parameter. Also adds `this.workspaceContext = null` in the error catch block (previously `workspaceContext` could remain stale on error). This is prerequisite cleanup for Bug 3.
+
+### Bug 3: Context Guard Blocking Workspaces (e2202c9c) — APPROVE
+
+**Problem**: In `ChatSettingsModal`, the guard `if (workspace?.context)` blocked workspace selection for any workspace without a populated `context` field. Since `context` is optional on `IndividualWorkspace`, most workspaces created through normal UI flows don't have it. The commit title claims 14/21 workspaces were blocked.
+
+**Fix**: Removes the guard. Now calls `setWorkspaceContext(workspaceId)` directly (safe after Bug 2 made the method self-contained with its own DB lookup).
+
+**Assessment**: Significant real bug. The guard was acting as an accidental filter.
+
+---
+
+## Summary Table
+
+| Item | Verdict | Source |
+|------|---------|--------|
+| Schema migrations v12-v19 | **REJECT** | PR #97 baggage |
+| Action bar (Insert/Append/Create File) | **REJECT** | PR #97 baggage |
+| Orphaned JSONL pruning | **REJECT** | PR #97 baggage |
+| Whitespace noise (~1,543 lines) | **REJECT** | PR #97 baggage |
+| ProviderHttpClient fix | **Accept separately** | PR #97 baggage (real fix) |
+| G-W1: Cheap restore | **APPROVE** | New in #104 |
+| G-W2: Slim workspace header | **APPROVE** | New in #104 |
+| G-W3: First-message full context | **APPROVE** | New in #104 |
+| G-W4: Dead fetch removal | **APPROVE** | New in #104 |
+| Bug: G-W3 flag consumed at init | **APPROVE** | New in #105 |
+| Bug: Redundant context param | **APPROVE** | New in #105 |
+| Bug: Context guard blocking workspaces | **APPROVE** | New in #105 |
+
+---
+
+## Recommendation
+
+**Neither PR can be merged as-is.** Both carry the full PR #97 payload (schema migrations, action bar, JSONL pruning, whitespace noise) which was previously rejected.
+
+**The new workspace optimization work (G-W1 through G-W4) and the 3 bug fixes in PR #105 are all sound and should be accepted** — but they need to be extracted into a clean PR that:
+
+1. Is rebased on current `main` (v5.6.10)
+2. Contains ONLY the workspace optimization commits and bug fixes
+3. Excludes all PR #97 baggage (schema migrations, action bar, JSONL pruning, whitespace)
+
+**Suggested path forward**: Ask Midway65 to cherry-pick commits `da5520e3` through `6bd34523` onto a fresh branch from `main`, or offer to do the extraction ourselves. The workspace work is ~8 commits touching only 3-5 files — a clean, reviewable PR.
+
+### If accepting PR #105 as "the one PR" (supersedes #104):
+
+The 3 extra commits in #105 fix real bugs introduced by #104's workspace changes. PR #104 without #105 has a broken G-W3 mechanism (flag consumed at init) and a workspace selection guard that blocks most workspaces. **PR #105 is strictly better than #104.**
+
+Close #104 in favor of #105, then request the baggage extraction before merge.