feat(plugin): prototype TypeScript candidate normalization - #643
feat(plugin): prototype TypeScript candidate normalization#643mldangelo-oai wants to merge 1 commit into
Conversation
|
@codex review |
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: f2a088a5bb
ℹ️ 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 (inputs.includes(output)) | ||
| throw new Error("--out: must not also be an input"); | ||
| if (output === scopePath) { | ||
| throw new Error("--out: must not replace --in-scope-files"); |
There was a problem hiding this comment.
On Windows, path identity is case-insensitive, but these string comparisons are case-sensitive and resolveAllowMissing() preserves the caller's spelling. If an existing input or scope file is passed as --out with different drive, directory, or filename casing, the check is bypassed and the final rename replaces that protected file; Python's Path comparison rejects the same invocation. Compare canonical Windows path keys or otherwise use case-insensitive path identity here.
AGENTS.md reference: sdk/typescript/AGENTS.md:L22-L27
Useful? React with 👍 / 👎.
| function positiveLine(value: unknown, field: string): number { | ||
| if (typeof value !== "number" || !Number.isInteger(value) || value < 1) { | ||
| throw new Error(`${field}: expected a positive integer`); | ||
| } | ||
| return value; |
There was a problem hiding this comment.
When model output encodes a line as 1.0 or 1e0, JSON.parse produces the number 1, so this check accepts it and writes a candidate; the Python oracle parses those tokens as floats and rejects them because they are not int values. This breaks the documented rejected-input parity during migration and can cause previously invalid candidate rows to enter scan results, so the JSON token type must be preserved or validated before coercion.
AGENTS.md reference: AGENTS.md:L21-L24
Useful? React with 👍 / 👎.
| const entrypoint = process.argv[1]; | ||
| if ( | ||
| entrypoint !== undefined && | ||
| import.meta.url === pathToFileURL(resolve(entrypoint)).href | ||
| ) { |
There was a problem hiding this comment.
When this script or any parent directory is invoked through a filesystem symlink, Node resolves import.meta.url to the real module while process.argv[1] retains the symlink spelling, so this condition is false and the process exits successfully without reading candidates or producing output. The Python helper works through the same symlink, so canonicalize both paths before comparing them to preserve executable behavior.
AGENTS.md reference: AGENTS.md:L21-L24
Useful? React with 👍 / 👎.
| async function runCli(): Promise<void> { | ||
| if (process.argv.slice(2).some(isHelpArgument)) { | ||
| console.log(HELP); |
There was a problem hiding this comment.
If --help or -h follows an option whose value is missing, such as --out --help or --input --help, this pre-scan prints help and exits successfully. The Python CLI instead reports the missing value and exits with status 2, so malformed automation can be silently treated as successful during migration; process help in argument order rather than bypassing parsing whenever it appears.
AGENTS.md reference: AGENTS.md:L28-L31
Useful? React with 👍 / 👎.
1 participant
Summary
Add a side-by-side TypeScript prototype for candidate normalization while preserving the existing Python implementation as the production entrypoint.
Changes
Testing
pnpm run typespnpm run formatbun test --timeout 30000 --seed 12345 tests-ts/normalize-candidates.test.ts tests-ts/normalize-candidates-filesystem.test.ts tests-ts/normalize-candidates.property.test.ts tests-ts/skeleton.test.ts: 26 passed, one Windows-only test skipped on macOS.bun test --timeout 30000 --seed 12345 ./tests-ts: 1,787 passed, 31 platform-specific skips, and zero failures on the previously verified base before the final refresh frommain.pnpm pack --pack-destination <temporary-directory>andnode scripts/check-package.mjs <archive>: 270 archive entries, installed SDK import, NodeNext types, CLI, 114 bundled plugin files, bundled Codex, and nested-worker smoke checks passed on the refreshed base.Risk and rollout
The Python normalizer remains the production implementation. The TypeScript helper is bundled alongside it for parity testing, with no public CLI changes or production cutover. Windows-specific parity coverage will run in pull-request CI.
Public disclosure review