Desktop: Phase 3 (step 2) broker serves /api/procedures - #71
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (3)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthrough
ChangesProcedures Endpoint
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: e14d998394
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| const out = [] | ||
| for (const p of iterSops(sopDir)) { | ||
| let m | ||
| try { m = parseFrontmatter(fs.readFileSync(p, 'utf8')) } catch (_) { continue } // skip unreadable SOP |
There was a problem hiding this comment.
When a SOP file contains invalid UTF-8 bytes, fs.readFileSync(p, 'utf8') silently replaces them with �, so this catch never runs. The FastAPI implementation this replaces catches UnicodeDecodeError and skips that SOP (covered by test_procedures_skips_unreadable_sop), so after the broker starts serving /api/procedures itself a corrupt/non-UTF-8 .md with frontmatter is returned to the UI instead of being omitted, breaking endpoint parity and exposing an unusable procedure.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Good catch, fixed. Node fs.readFileSync(p,"utf8") substitutes U+FFFD on invalid bytes (no throw), so a corrupt SOP/queue file would be parsed garbled and INCLUDED, where Python read_text(encoding="utf-8") raises and _procedures/_queue skip it. Added readTextStrict() using TextDecoder({fatal:true}) so the read throws and the caller's try/catch skips, matching Python. Test added (a non-UTF-8 ops/ and queue/ file are skipped).
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@desktop/store.js`:
- Around line 54-71: The collectMd function currently traverses into all
directories and only filters out SKIP_DIRS later in iterSops, causing
unnecessary traversal of large directories like archive and active-runs. Modify
collectMd to accept SKIP_DIRS as a parameter and check if each directory is in
SKIP_DIRS before recursing into it. Update the recursive call in collectMd to
pass SKIP_DIRS along, and update the call to collectMd in iterSops to pass
SKIP_DIRS as the third argument. This will prevent traversal into excluded
directories during collection rather than filtering them after collection is
complete.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: f52d440e-a85b-4e3a-a2ab-d775b2dacee3
📒 Files selected for processing (3)
desktop/broker.jsdesktop/store.jsdesktop/store.test.js
The broker now also answers /api/procedures from the SOP files + runs.jsonl directly: the procedure list with each SOP's derived autonomy level, cost estimate (median of prior 'ok' runs), and the draft/interactive/needs_inputs flags -- porting iter_sops' skip rules, autonomy_level_from_meta, and _cost_estimates' per-SOP median into store.js. Verified by SEMANTIC parity against the live FastAPI (parsed JSON deep-equal, so a whole-dollar cost serialized as 1 here vs 1.0 in Python -- identical after JSON.parse, which is what the SPA does -- doesn't read as a mismatch). plate + queue still parity-clean. Still forwarding: settings (reads an environment-detected terminal), pending, and the liveness-bearing reads + SSE (Phase 5 flock/pid migration). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
e14d998 to
e1c3b2e
Compare
|
Adversarial self-review hardening (folded into the latest push), in addition to the bot fixes above:
|
1 participant
Continues Phase 3: the broker now answers
/api/proceduresitself, alongside/api/plate+/api/queue.What moved
desktop/store.jsports three Python helpers so the broker can build the procedure list directly from the SOP files +runs.jsonl:iter_sopsskip rules (rglob*.md, skip runtime dirs /INDEX.md/_template.md/DIGEST.md/ dotfiles),autonomy_level_from_meta(explicit field → else interactive→with_me → else status→on_its_own/prepare_ask),_cost_estimatesper-SOP median of priorokruns (numeric sort, even/odd median, non-ok/bool/negative excluded).Each row:
id, title, draft, interactive, needs_inputs, cost {estimate,n}|null, autonomy, sorted by title.Parity
Verified by semantic parity against the live FastAPI (parsed-JSON deep-equal). A whole-dollar cost serializes as
1here vs1.0in Python — identical afterJSON.parse(what the SPA does), so it's not a real mismatch; the check compares parsed values, not bytes. plate + queue stay parity-clean.Still forwarding
settings(reads an environment-detected terminal, not a pure static read),pending, and the liveness-bearing reads + the SSE live mirror (the Phase 5 flock/pid migration).Tests
node --test: 19 (added a procedures unit test: autonomy derivation, cost median, INDEX skip, title sort) + the live semantic parity check.🤖 Generated with Claude Code
Summary by CodeRabbit