Shrink Compact workflow prompts and fix evidence false positives - #326
Conversation
Compact now gets a brief execution contract and a shorter progress_update.workflowReconciliation schema. The evidence kernel matches AX-truncated metadata, keeps valid fields when one name is unknown, emits live required=false, and fail-closes only on form-relevant depth or iframe omission. Co-authored-by: Emre Sokullu <esokullu@gmail.com>
Co-authored-by: Emre Sokullu <esokullu@gmail.com>
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. |
![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: cfa21797e6
ℹ️ 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".
| if (got.endsWith('...') && got.length === 63) { | ||
| const prefix = got.slice(0, 60); | ||
| return want.startsWith(prefix) && want.length > 60; |
There was a problem hiding this comment.
When a saved YouTube metadata value exceeds 60 characters, this accepts any observed value sharing the first 60 characters with the requested value. For example, if an existing description has the same opening sentence but differs afterward because the new value was not fully saved, _workflowMetadataRequirementsMatchInventory can still produce saved_state_with_exact_metadata_readback and report success. The inventory needs an untruncated app-owned value for verification rather than treating the model-facing AX abbreviation as exact evidence.
Useful? React with 👍 / 👎.
| if (!value || typeof value !== 'object' || Array.isArray(value)) continue; | ||
| if (!Object.prototype.hasOwnProperty.call(value, 'value')) continue; | ||
| const field = this._workflowMetadataFieldKey(value.field); | ||
| if (!Object.prototype.hasOwnProperty.call(value, 'value')) return []; | ||
| const expectedValue = this._workflowMetadataValue(value.value); | ||
| if (!field || requirements.has(field)) return []; | ||
| requirements.set(field, { field, value: expectedValue }); | ||
| if (!field || requirements.has(field)) continue; | ||
| requirements.set(field, { field, value: this._workflowMetadataValue(value.value) }); |
There was a problem hiding this comment.
When the classifier returns at least one valid workflow field plus a malformed, unknown, or duplicate field, this silently drops the latter and binds submission verification only to the surviving subset. A request for multiple metadata changes can therefore be declared successfully saved even though the discarded field was never verified. Keep the valid entries if desired, but retain an invalid/incomplete marker so terminal verification cannot pass until all classifier requirements are accounted for.
Useful? React with 👍 / 👎.
| const noisy = !!frame?.error | ||
| || frame?.truncated === true | ||
| || (frame?.ok === false && matchCount !== 0); | ||
| if (frameItems.length === 0 && noisy) continue; |
There was a problem hiding this comment.
When a multi-frame form read succeeds for some frames but a relevant application frame errors before returning any controls, this removes the failed frame from documents exactly like an ad frame. The remaining frames can then make evidence.complete true and allow reconciliation despite missing every control from the failed frame; nothing here verifies that the omitted URL is third-party or decorative. Only known-noise frames should be excluded, while an unclassified failed or truncated frame must keep the inventory incomplete.
Useful? React with 👍 / 👎.
Truncated AX values now bind value_len and value_fp so a shared 60-character prefix is not exact saved-state evidence. Failed same-site or urlFilter-targeted frames stay in inventory documents. Discarded classifier metadata fields keep verification incomplete without wiping the rest of the list. Co-authored-by: Emre Sokullu <esokullu@gmail.com>
|
@codex pls review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 96a50e57a2
ℹ️ 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".
| if (isInventoryFormControl) { | ||
| let required = false; | ||
| try { | ||
| required = !!el.required || el.getAttribute('aria-required') === 'true'; | ||
| } catch {} | ||
| line += required ? ' required=true' : ' required=false'; |
There was a problem hiding this comment.
In both browser tree builders, every form-like control without native required or aria-required="true" is now emitted as required=false. Many React/custom forms enforce required fields through application validation without either DOM attribute, but workflowRequiredRowsAreProcessed treats this explicit false value as permission to mark the row skipped. A prepare-form or prepare-application workflow can therefore reconcile successfully while omitting an application-required field; preserve an unknown state unless optionality is explicitly established.
Useful? React with 👍 / 👎.
| if (frameItems.length === 0 && noisy | ||
| && !this._workflowIframeFrameIsApplicationScoped(frameUrl, pageUrl, args?.urlFilter)) { | ||
| continue; |
There was a problem hiding this comment.
When the actual embedded form is hosted on a different domain and a broad iframe_read is made without urlFilter, an injection/error result with no returned controls is classified as noise solely because its hostname differs from the page. The frame is then removed from documents, so successful sibling documents can make the inventory complete even though this frame was never inspected. Fresh evidence in this revision is the new _workflowIframeFrameIsApplicationScoped hostname heuristic; cross-origin embedded forms are supported elsewhere in the executor and are not inherently decorative, so an unclassified failed frame must remain incomplete.
Useful? React with 👍 / 👎.
| const axInventoryValue = (raw) => String(raw ?? '') | ||
| .replace(/\r\n?/g, '\n') | ||
| .replace(/[\u0000-\u0008\u000b\u000c\u000e-\u001f\u007f]/g, '') | ||
| .trim(); |
There was a problem hiding this comment.
For values longer than 60 characters containing an NFKC-changing character, such as full-width Latin text or compatibility ligatures, the AX builder hashes the raw DOM string while _workflowAxValueMatchesExpected hashes the NFKC-normalized requested value. Even when the exact requested value was saved, value_fp will differ and saved-state verification will fail in both Chrome and Firefox. Normalize the inventory value identically before computing both value_len and value_fp.
Useful? React with 👍 / 👎.
Emit required= only for native required or an explicit aria-required value, so React/app validation cannot be skipped as optional. Hash AX metadata values after the same NFKC normalization used by verification. Omit empty third-party iframe noise only when another frame already inventoried form controls; a lone failed cross-origin application frame stays incomplete. Co-authored-by: Emre Sokullu <esokullu@gmail.com>
|
@codex pls review |
|
Codex Review: Didn't find any major issues. You're on a roll. Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
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". |
204a063
into
codex/report-driven-adapter-workflows
2 participants
Implements the Compact shrink + evidence false-positive plan against PR #320 (
codex/report-driven-adapter-workflows).Track A — Compact prompt shrink
formatAdapterWorkflowExecutionPolicy(siteWorkflow, { form: 'brief' | 'full' })— Compact injects the brief contract (job, success, submit,filter:"all"ledger, optional skip); Mid/Full keep the full essay.progress_update.workflowReconciliationschema descriptions. The field is still present.Track B — Evidence kernel
Fail closed on form-relevant omission, not page noise:
value=(even when it equals the name). Truncated values also emitvalue_len/value_fpafter the same NFKC normalization used by verification; matching requires prefix and those bindings, not prefix alone.iframe_reademitrequired=trueonly for native/aria-required="true"controls, andrequired=falseonly whenaria-required="false". Missingrequiredstays unknown and cannot skip (React/app validation is not inferred as optional).depthTruncated— set only when an omitted descendant would have been included.urlFilter-targeted frames stay incomplete.Chrome/Firefox shared modules stay byte-identical.
Tests
Adds Compact brief/schema coverage plus regressions for the evidence kernel and review tightenings. Existing full-form policy assertions stay on
form: 'full'. Localnode test/run.js: 2123 passed; the only failure is the pre-existing missingdist/webbrain-chrome-33.5.0.zippackaging check.