ANS-1118: Add Semantic Optimization CLI commands - #34
Conversation
Greptile SummaryAdds Semantic Optimization CLI support across the complete run lifecycle.
Confidence Score: 5/5The PR appears safe to merge; both previously reported contract defects are fully addressed and regression-tested. Promotion now requires and sends Important Files Changed
Flowchart%%{init: {'theme': 'neutral'}}%%
flowchart TD
Start[optimize start] --> Run[Optimization run]
Run --> Inspect[ list / get ]
Run --> Control[ pause / resume / stop ]
Run --> Clone[clone]
Run --> Review{Review decision}
Review -->|approve| Approved[Approved proposal]
Review -->|reject| Rejected[Rejected proposal]
Approved --> Promote[promote with frozen base hash]
Promote --> CAS{Active hash matches?}
CAS -->|yes| Active[Promote semantic layer]
CAS -->|no| Conflict[Reject stale promotion]
Reviews (2): Last reviewed commit: "fix: require safe optimization inputs" | Re-trigger Greptile |
| if (["pause", "resume", "stop", "promote"].includes(command)) { | ||
| return requestAndPrint(client, "POST", `${base}/${encodeURIComponent(runId)}/${command}`, parsed, io, { | ||
| body: { reason: firstValue(parsed.flags.reason) }, | ||
| }); | ||
| } |
There was a problem hiding this comment.
promote is documented as using an optimistic base-hash check, but this branch sends only reason and provides no flag or request-data path for the expected hash. Callers therefore cannot supply the concurrency token needed to reject a promotion against a stale semantic layer, so promotion will either fail API validation or bypass the documented stale-base protection.
There was a problem hiding this comment.
Resolved in f4f0304: optimize promote now requires a base-hash flag and sends it as expected_active_sha256. The README and a request-payload regression test cover the atomic compare-and-swap contract.
| if (!Object.values(payload.models || {}).every(Boolean)) { | ||
| throw usage("optimize start requires --model or every phase-specific model flag"); | ||
| } |
There was a problem hiding this comment.
Structured data shallowly replaces the default models object, while this check validates only the values that remain. Because Object.values({}).every(Boolean) is true, --data '{"models":{}}' bypasses the model requirement and sends a run with no phase models, leaving the API to reject or mishandle the request.
There was a problem hiding this comment.
Resolved in f4f0304: validation now checks the explicit required keys eval_execution and proposal and rejects missing, non-string, or blank values. A regression test covers an empty structured models object.
|
@greptileai Please re-review the latest commit (f4f0304). Both findings from the previous review have been addressed and regression-tested. |
|
🎉 This PR is included in version 0.2.1 🎉 The release is available on: Your semantic-release bot 📦🚀 |
Summary
answerlayer optimizecommands for starting, listing, inspecting, pausing, resuming, stopping, cloning, reviewing, and promoting Semantic Optimization RunsVerification
npm test: 68 passedCompanion change
Linear: ANS-1118