feat: Commander V1 autocomplete scaffold - #1
Conversation
![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: 01d13f1e0f
ℹ️ 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 express = require('express'); |
There was a problem hiding this comment.
Because package.json still declares "type": "module", running the configured npm start evaluates this file as an ES module and immediately throws ReferenceError: require is not defined at this line, before the server can bind a port. Convert this file and its exports to ESM or explicitly use CommonJS files/configuration so every API endpoint remains reachable.
Useful? React with 👍 / 👎.
| const express = require('express'); | ||
| const path = require('path'); | ||
| const helmet = require('helmet'); | ||
| const rateLimit = require('express-rate-limit'); | ||
| const cors = require('cors'); |
There was a problem hiding this comment.
A fresh checkout followed by npm install will not install any of these packages: the unchanged manifest has no dependencies section or lockfile, while this server now requires Express and its middleware at startup. Once the module-format issue is corrected, startup will instead fail with MODULE_NOT_FOUND unless these production dependencies are added to the manifest.
Useful? React with 👍 / 👎.
| const { orchestrate } = require('./orchestrator'); | ||
| const { combineSelected } = require('./combine'); |
There was a problem hiding this comment.
The target commit contains no src/orchestrator.js or src/combine.js (confirmed from its tracked file tree), so these local imports cannot resolve. Even with the module format and npm dependencies fixed, Node will fail while loading the server rather than serving either /api/suggest or /api/combine; include the implementations or retain the existing request handlers.
Useful? React with 👍 / 👎.
| app.post('/api/suggest', async (req, res) => { | ||
| try { | ||
| const body = req.body || {}; | ||
| const { promptPartial, cursorIndex = null, context = {}, max_suggestions = 5, all = false } = body; |
There was a problem hiding this comment.
When a deployment sets the documented MAX_SUGGESTIONS environment variable and a caller omits max_suggestions, this destructuring default always passes 5 to orchestrate, bypassing the setting. The previous handler used the environment-derived default, so restore that behavior to keep configured fan-out and model cost effective.
Useful? React with 👍 / 👎.
| if (!promptPartial || typeof promptPartial !== 'string') return res.status(400).json({ error: 'promptPartial (string) required' }); | ||
| if (max_suggestions <= 0) return res.status(400).json({ error: 'max_suggestions must be positive' }); |
There was a problem hiding this comment.
Requests with prompt text between the former 20,000-character limit and the 64 KB JSON-body limit now pass this validation and are forwarded to generation instead of receiving a 400 response. In deployments with model generation enabled, those oversized requests can multiply token cost and latency across the orchestrated agents; retain an explicit prompt-length limit here.
Useful? React with 👍 / 👎.
| if (req.method === "OPTIONS") return json(res, 204, {}); | ||
| if (req.method !== "POST" || req.url !== "/api/suggest") { | ||
| return json(res, 404, { error: "Not found" }); | ||
| const suggestions = await orchestrate({ promptPartial, cursorIndex, context, max_suggestions, all }); |
There was a problem hiding this comment.
The newly added isDangerous function is never imported or called anywhere in the target commit (verified with a repository-wide search), and this route forwards promptPartial directly to orchestration. Consequently, inputs matching the filter's dangerous patterns are generated normally, so the added safety filter has no effect until it is applied in the request and/or output path.
Useful? React with 👍 / 👎.
| app.use(limiter); | ||
|
|
||
| // serve static frontend | ||
| app.use(express.static(path.join(__dirname, '..', 'public'))); |
There was a problem hiding this comment.
The target commit tracks no public/ directory or frontend files, so this middleware has nothing to serve: a browser request for / falls through and receives Express's 404 response. Add the referenced static assets (or remove this route until they exist) so the newly introduced frontend entry point is usable.
Useful? React with 👍 / 👎.
1 participant
Summary
Implements the V1 Commander autocomplete scaffold on
scaffold/mvp.Included
Verification
CI should be treated as the automated gate. Manual browser verification remains environment-dependent.
V1 product boundary
Commander is an intelligent autocomplete layer for human thoughts, requests, and AI instructions. Autocomplete is the core UX; prompt improvement is a supporting capability.