Skip to content

fix(omp): allow quoted # in OMP-managed YAML (treat # as comment only outside quotes) - #251

Closed
luvs01 wants to merge 5 commits into
devfrom
codex/fix-quoted-#-in-omp-model-ids
Closed

luvs01 wants to merge 5 commits into
devfrom
codex/fix-quoted-#-in-omp-model-ids

Conversation

@luvs01

@luvs01 luvs01 commented Aug 11, 2026 •

Copy link
Copy Markdown
Owner

Motivation

  • The OMP YAML writer conservatively rejected any # byte inside a managed block as an inline comment, which also blocked legitimate generated YAML that contains # inside quoted scalars (for example model IDs or names like "provider/model#variant").
  • This made normal integration refresh/disable operations fail when model IDs or labels include #, creating an availability/integrity problem for OMP-managed blocks.

Description

  • Replace the naive hasInlineComment(line: string) implementation with a scanner that tracks single- and double-quoted YAML scalars and treats # as an inline comment only when it appears outside quotes and after YAML separation whitespace; the scanner also handles escaped double-quote sequences and doubled single quotes. (change in src/integrations/omp-yaml-source.ts).
  • Preserve the conservative refusal behavior for true nested inline comments so user-authored comments are still protected by the writer's safety checks. (existing childEnd / container logic remains guarded.)
  • Add focused regression tests that assert refresh and disable succeed for quoted # scalars while still refusing real nested inline comments. (added test cases in tests/omp-yaml-source-inline-comments.test.ts).

Testing

  • Ran the focused test file with Bun: node_modules/@oven/bun-linux-x64-baseline/bin/bun test tests/omp-yaml-source-inline-comments.test.ts, which passed (4/4 tests passed).
  • Ran typecheck with Bun/tsc: node_modules/@oven/bun-linux-x64-baseline/bin/bun run typecheck, which completed successfully.
  • Attempted a full test run node_modules/@oven/bun-linux-x64-baseline/bin/bun run test; the full suite run exposed unrelated pre-existing failures/timeouts in other tests (lab-live-review regressions and a sideband timeout), so the change was validated with the focused regression and typecheck but a green full-suite run was blocked by unrelated existing failures.

Codex Task


Devin Review

@github-actions github-actions Bot added the bug Something isn't working label Aug 11, 2026
@coderabbitai

coderabbitai Bot commented Aug 11, 2026 •

Copy link
Copy Markdown

Important

  • 🔍 Trigger review

This repository does not receive automatic reviews because it has fewer than 10 stars.

⚙️ Run configuration

Configuration used: Repository: luvs01/opencodex/.coderabbit.yaml

Review profile: ASSERTIVE

Plan: Advanced

Run ID: 8ef2df51-c735-4fd3-ae07-fac3e028a394


Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@github-actions

Copy link
Copy Markdown

✅ Deterministic PR hygiene checks passed.

@github-actions

github-actions Bot commented Aug 11, 2026 •

Copy link
Copy Markdown

✅ READY

  • all PR quality gates passed.

Hygiene

✅ Deterministic PR hygiene checks passed.

@github-actions github-actions Bot changed the title fix(omp): allow quoted # in OMP-managed YAML (treat # as comment only outside quotes) [WRONG BRANCH] fix(omp): allow quoted # in OMP-managed YAML (treat # as comment only outside quotes) Aug 11, 2026
@github-actions
github-actions Bot marked this pull request as draft August 11, 2026 00:55

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: e0fdbaf26a

ℹ️ 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".

Comment thread src/integrations/omp-yaml-source.ts Outdated
else quote = null;
continue;
}
if (character === "'" || character === "\"") quote = character;

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge Restrict quote tracking to quoted-scalar starts

When an OMP-managed plain scalar contains an apostrophe or double quote before a real inline comment—for example, name: user's model # keep this note, which is valid YAML—hasInlineComment enters quote mode even though YAML treats that quote as ordinary plain-scalar content. The whitespace-separated # is therefore missed, so refresh or disable proceeds and silently deletes the user-authored comment instead of returning null. Only enter quote state where a quoted scalar can syntactically begin (or use a YAML lexer/CST), and add regression coverage for quoted characters inside plain scalars.

AGENTS.md reference: src/AGENTS.md:L10-L10

Useful? React with 👍 / 👎.

@luvs01 luvs01 closed this Aug 19, 2026
@luvs01 luvs01 reopened this Aug 19, 2026
@luvs01 luvs01 changed the title [WRONG BRANCH] fix(omp): allow quoted # in OMP-managed YAML (treat # as comment only outside quotes) fix(omp): allow quoted # in OMP-managed YAML (treat # as comment only outside quotes) Sep 3, 2026
@luvs01
luvs01 changed the base branch from main to dev September 3, 2026 06:24
@github-actions
github-actions Bot marked this pull request as ready for review September 3, 2026 06:33
devin-ai-integration[bot]

This comment was marked as resolved.

Co-Authored-By: Epinephrine <luvs01@hanmail.net>
@devin-ai-integration

Copy link
Copy Markdown

Fixed in 8043a409e. hasInlineComment now only opens quote mode where a scalar may begin (after key:, - , a flow indicator, or an anchor/alias/tag) — quotes embedded in a plain scalar like user's model or model"beta are treated as content, so a following # is seen and the mutation refuses instead of silently deleting the comment. Added refresh + disable regressions for both shapes in tests/clients/omp-yaml-source-inline-comments.test.ts.

devin-ai-integration[bot]

This comment was marked as resolved.

@devin-ai-integration

Copy link
Copy Markdown

Already addressed at head by 8043a40 (fix(integrations): only open YAML quote mode where a scalar may begin). hasInlineComment now tracks plain-scalar context — a ' or " opens quote mode only where a scalar may begin, so # after an apostrophe or embedded " is still detected. Regressions for user's model # note and model"beta # note (both refresh and disable refusal) are in tests/clients/omp-yaml-source-inline-comments.test.ts.

…range comment scan

Co-Authored-By: Epinephrine <luvs01@hanmail.net>
devin-ai-integration[bot]

This comment was marked as resolved.

…rator blanks

- A '- ' indicator opens a new node: carrying scalarStarted across it made a
  ' #' inside a later quoted item read as a comment and refuse the patch
- A blank after a plain scalar folds only when deeper content follows; a
  shallower next line means the blank is a separator and stays outside the
  leaf range instead of being deleted on refresh/remove

Co-Authored-By: Epinephrine <luvs01@hanmail.net>
@luvs01

luvs01 commented Sep 24, 2026

Copy link
Copy Markdown
Owner Author

Closing: submitted upstream as lidge-jun#5787, rebased onto current dev and verified — bun test tests/clients/omp-yaml-source-inline-comments.test.ts 18 pass. Nothing left to merge here.

@luvs01 luvs01 closed this Sep 24, 2026
@github-actions
github-actions Bot deleted the codex/fix-quoted-#-in-omp-model-ids branch October 14, 2026 12:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

aardvark bug Something isn't working codex

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant