Conversation
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: f52610ffde
ℹ️ 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 c=await load(file);return mutate(c,async()=>{ | ||
| const before=await snapshot(c),base=await readJSON(join(c.state,'baseline.json'),null); | ||
| if(!base||base.skillHash!==before.skillHash||base.conditions!==before.conditions)throw Error('Save a baseline for the current skill and conditions first'); | ||
| const snap=await snapshot(c,candidate),result=await execute(c,snap); |
There was a problem hiding this comment.
Support staging in assistant-only mode
When the config comes from the default init flow, runner.command is absent because evaluations use prepare/ingest; however, stage unconditionally calls execute, which throws No runner command. Consequently, users following the documented assistant workflow can establish a baseline but cannot test or stage any manual candidate without switching to an automated command runner. Add a candidate prepare/ingest path or otherwise allow assistant-supplied candidate outputs.
Useful? React with 👍 / 👎.
| const run={id,createdAt:new Date().toISOString(),kind,runner:config.runner.label,engineVersion:'0.1.0',skillPath:config.skill,qaSource:snap.suite.source??null,coverage:snap.suite.coverage??'Only the supplied cases and checks; broader task quality is unverified',conditions:snap.conditions,skillHash:snap.skillHash,...score(snap.suite,response)}; | ||
| const baseline=await readJSON(join(config.state,'baseline.json'),null);run.comparison=compare(baseline,run); | ||
| await atomic(join(config.state,'runs',id+'.json'),{...run,request,response,skill:snap.skill,suite:snap.suite}); | ||
| await atomic(join(config.state,'latest.json'),run); |
There was a problem hiding this comment.
Keep exploratory candidate runs out of latest state
During stage and every loop evaluation, execute reaches saveRun, which overwrites latest.json even though the candidate has not been approved or applied. After staging an ineligible candidate—or after a loop ends with a rejected trial—status, report, and comparison displays therefore describe the exploratory candidate as the latest result for the still-unchanged active skill, corrupting the drift signal used for review. Preserve the active-skill latest run separately from candidate evaluations.
Useful? React with 👍 / 👎.
| let request; | ||
| try {if(line.length>5_000_000)throw Error('Message too large');request=JSON.parse(line);} | ||
| catch {send({jsonrpc:'2.0',id:null,error:{code:-32700,message:'Invalid JSON message'}});continue;} | ||
| if(!Object.hasOwn(request,'id'))continue; |
There was a problem hiding this comment.
Reject non-object JSON-RPC messages without exiting
If an MCP peer sends valid JSON that is not an object, especially null, Object.hasOwn(request, 'id') throws outside the request error handling and terminates the stdio server. A malformed or probing client message can therefore disconnect the entire integration instead of receiving an invalid-request response; validate that the parsed value is a non-null object before accessing it.
Useful? React with 👍 / 👎.
Adds a Skill Loop plugin for testing reusable skills against explicit QA sources, detecting effectiveness drift, and reviewing revisions. It shares bounded iteration mechanics with GTM Autoresearch while preserving each adapter’s scoring and permitted changes.
The workshop now uses the actual Humanizer 2.9.1 skill exported from Jordaaan’s Claude setup, with its MIT license preserved. A single GitHub-link request in regular Claude Chat initializes the cases, produces real rewrites, scores their literal facts and selected unwanted strings, and returns an interactive HTML artifact. The artifact matches the Skill Loop homepage’s Organized AI design and Jordaaan branding, shows source/output comparisons, and supports skill-version review and untested drafts. The separate offline demo is explicitly a punctuation teaching adaptation, not a live Humanizer benchmark.
Active skills change only through reviewed approval or an explicit version choice. Drafts require fresh tests; the HTML cannot apply changes itself. Missing suites remain untested. Literal checks do not establish overall prose quality or semantic preservation. The current Chat route supports prepare/ingest evaluation; automated candidate staging requires a configured runner.
Validation: 41 shared/engine tests passed, followed by 22 Skill Loop tests including the saved-baseline display regression. Plugin and skill validation, clean extracted-package execution, and the three-click browser fallback passed. Independent review checked source fidelity, negative-check behavior, scorer versioning, HTML escaping and visual consistency. Live regular Claude Chat ran the full Humanizer suite at 15/15, saved the baseline, preserved the source skill and rendered the branded interactive artifact; the refreshed artifact was visually verified. Earlier live Codex fixture testing also succeeded. Cowork execution and fresh participant plugin installation remain outside these verified paths. Verification is runnable through scripts/verify-skill-loop.sh.