Skip to content

fix(codex): map the collaboration_mode prompt layer - #5298

Closed
luvs01 wants to merge 1 commit into
lidge-jun:devfrom
luvs01:fix/collaboration-mode-layer
Closed

luvs01 wants to merge 1 commit into
lidge-jun:devfrom
luvs01:fix/collaboration-mode-layer

Conversation

@luvs01

@luvs01 luvs01 commented Sep 20, 2026

Copy link
Copy Markdown
Collaborator

Summary

  • The prompt-text probe listed collaboration in UNMAPPED_LAYER_IDS, so a rendered <collaboration_mode> section was reported as unmapped even though Codex wraps collaboration instructions in that tag when the active mode emits them.
  • Map collaboration to collaboration_mode in LAYER_SECTION_TAGS and extract the section-to-layer projection into a testable mapSectionsToLayers seam.

Verification

  • bun test tests/codex-integration/codex-prompt-text-probe.test.ts (43 pass, 1 pre-existing skip)
  • bun x tsc --noEmit

Checklist

  • Scope stays focused and avoids unrelated cleanup.
  • Docs or release notes were updated when needed.
  • Security-sensitive changes were reviewed for secrets, auth, and unsafe defaults.

Summary by CodeRabbit

  • Bug Fixes

    • Collaboration content is now correctly recognized when rendered, including its text and size.
    • Empty collaboration sections now report as not rendered instead of unmapped.
  • Tests

    • Added coverage for rendered and absent collaboration sections.

Review readiness checklist

This PR stays in draft until every box below is ticked. Tick all four boxes once the requirements are met:

  • All CI tests are green on my local testing.

  • I pushed my PR to the latest dev commit.

  • I resolved all correct Codex and CodeRabbit findings.

  • My PR is ready for review.

@coderabbitai

coderabbitai Bot commented Sep 20, 2026

Copy link
Copy Markdown
Contributor

Review Change StackReview Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Repository: lidge-jun/opencodex/.coderabbit.yaml

Review profile: ASSERTIVE

Plan: Advanced

Run ID: 16ad7619-4ec4-47e2-9918-5761d57180bd

📥 Commits

Reviewing files that changed from the base of the PR and between 53725b0 and 7953e61.

📒 Files selected for processing (2)
  • src/codex/prompt-text-probe.ts
  • tests/codex-integration/codex-prompt-text-probe.test.ts

Included review availability: Your plan provides up to 10 included reviews per hour; 1 remains after this review.


📝 Walkthrough

Walkthrough

The prompt text probe now maps the rendered collaboration_mode section to the collaboration layer. The mapping logic is reusable through a test seam, and integration tests cover rendered and absent sections.

Changes

Collaboration prompt projection

Layer / File(s) Summary
Collaboration tag mapping
src/codex/prompt-text-probe.ts
LAYER_SECTION_TAGS maps collaboration to collaboration_mode. collaboration is removed from UNMAPPED_LAYER_IDS.
Section projection and validation
src/codex/prompt-text-probe.ts, tests/codex-integration/codex-prompt-text-probe.test.ts
mapSectionsToLayers replaces the inline projection in probePromptText and is exported through mapSectionsToLayersForTests. Tests verify rendered text, byte length, and the not-rendered result for an absent section.

Priority: ⬇️ Low

Estimated code review effort: 2 (Simple) | ~10 minutes

Change: Bug fix

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 50.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 2 functions across 2 files. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly and concisely describes the main change: mapping the collaboration prompt layer to the collaboration_mode section tag.
  • Fix all pre-merge checks with AI
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create a new PR

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
Contributor

Deterministic PR hygiene checks passed.

@github-actions github-actions Bot added the bug Something isn't working label Sep 20, 2026
@lidge-jun

Copy link
Copy Markdown
Owner

리뷰 · 우선순위 34 / 80

이 PR은 프롬프트 텍스트 프로브가 collaboration 레이어를 잘못 “매핑 없음(unmapped)”으로 보고하던 버그를 고칩니다. Codex가 협업 모드 안내를 실제로 넣을 때 <collaboration_mode> 태그로 감싸는데, 프로브 표에는 그 태그가 없고 UNMAPPED_LAYER_IDS에만 collaboration이 있어서, 화면에 글자가 있어도 “이 레이어는 아직 못 읽는다”고 나왔습니다.

이번 변경은 세 가지입니다. 첫째, LAYER_SECTION_TAGScollaboration → collaboration_mode를 넣습니다. 둘째, UNMAPPED_LAYER_IDS에서 collaboration을 뺍니다. 셋째, 섹션→레이어 매핑을 mapSectionsToLayers로 빼서 테스트에서 바로 확인할 수 있게 합니다. 테스트는 태그가 있을 때 본문·바이트가 맞고, 없을 때 not-rendered가 나오는 두 경우를 봅니다. 베이스는 dev입니다. 인벤토리 문서에도 Collaboration mode 태그가 <collaboration_mode>로 적혀 있어, 태그 이름 자체는 저장소 근거와 맞습니다. types.ts/config.ts 분할과 무관하고, 같은 주제로 닫을 다른 열린 PR도 보이지 않습니다.

라인 - src/codex/prompt-text-probe.ts LAYER_SECTION_TAGS.collaboration — 태그 이름은 인벤토리(001_prompt_layer_inventory.md)와 맞습니다. 다만 이 PR 본문에는 이번 머리에서 찍은 codex debug prompt-input 원문 조각은 없습니다. 파일 머리 주석 관례(“라이브 출력에서 읽었다”)를 믿는 수준입니다.
라인 - mapSectionsToLayers — 태그는 있는데 trim 후 본문이 비면 reason: "ok"에 빈 문자열입니다. 다른 매핑 레이어와 같은 기존 규칙이고, 이번 PR이 새로 만든 구멍은 아닙니다.
라인 - PR 체크리스트 — 네 칸이 모두 비어 있습니다. 코드 결함이라기보다 머지 전 절차가 아직입니다.
라인 - CI — 이 글을 쓰는 순간 label은 통과, enforce-target·hygiene는 대기였습니다. 초록을 한 번 더 확인하는 편이 좋습니다.

메인테이너의 판단이 필요한 지점

인벤토리·기존 empirical 메모만으로 collaboration_mode 매핑을 확정할지, 아니면 한 번 라이브 prompt-input 캡처를 PR에 붙여 달라고 할지. 협업 모드가 안내를 안 내는 환경에서는 계속 not-rendered로 보이는데, 그걸 UI에서 “꺼짐”과 어떻게 구분해서 설명할지(이미 다른 컨텍스트 의존 레이어와 같은 문제)는 제품 쪽 선택입니다.

너의 추천

방향이 맞고 범위도 좁습니다. 매핑·테스트 seam·회귀 테스트가 한 줄로 맞물려 있어서, CI가 초록이고 체크리스트를 채운 뒤 dev에 머지해도 됩니다. 라이브 캡처는 있으면 안심이 되지만, 없어도 이 PR을 막을 정도는 아닙니다. types/config 분할로 닫을 대상이 아닙니다.

이 댓글은 grok-bot이 작성했습니다

@github-actions

github-actions Bot commented Sep 20, 2026

Copy link
Copy Markdown
Contributor

✅ READY

  • all PR quality gates passed; the review readiness checklist is complete.

Review readiness checklist

  • ✅ All CI tests are green on my local testing.
  • ✅ I pushed my PR to the latest dev commit.
  • ✅ I resolved all correct Codex and CodeRabbit findings.
  • ✅ My PR is ready for review.

4/4 boxes ticked.

Automatic ready-for-review conversion failed; please mark the pull request ready manually if it is still a draft.
The review-ready label marks this PR as ready; review automation runs independently.
Maintainers notified: @lidge-jun @Ingwannu

@luvs01

luvs01 commented Sep 22, 2026

Copy link
Copy Markdown
Collaborator Author

Consolidated into #5503 in native Stack #5505.

Source head: 7953e61a7f48d4fdec21959e618a8038b436b57d. Replacement head: da8b575a51be2bb0c9b1c6d202c4400f07163edc. All 1 unique non-merge source commit(s) were matched to their cherry-picked replacement commits using stable Git patch IDs; attribution and provenance are preserved.

Closing this standalone PR as a duplicate review entry at the author's request. The change is not merged or released: the replacement remains draft, with remaining integration/CI/security-review work tracked there. Original branches are retained. The maintainer-cancelled CI is not treated as a pass.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working review-ready

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants