Skip to content

fix(codex): deduplicate unchanged effort clamp observations - #7005

Closed
Ingwannu wants to merge 2 commits into
devfrom
fix/7004-clamp-notice-s120
Closed

Ingwannu wants to merge 2 commits into
devfrom
fix/7004-clamp-notice-s120

Conversation

@Ingwannu

@Ingwannu Ingwannu commented Oct 12, 2026 •

Copy link
Copy Markdown
Owner

Summary

  • Closes ocx sync re-prints 'Removed unsupported reasoning efforts' + doctor hint on every sync, even when nothing changed #7004. Treat a Codex effort clamp as a persisted observation, not a per-sync heartbeat. Identical runtime path/version and order-independent removed-effort/affected-model sets leave the diagnostic bytes, inode and timestamp unchanged and do not repeat the notice.
  • Keep actual clamping and the per-projection callback unchanged. Changed observations are persisted/reported; clearing the clamp allows a later recurrence to be reported again. Remove the misleading sync-only doctor suggestion.
  • Reuse the existing diagnostic reader/atomic writer and schema; no new dependency, provider call or runtime configuration. Update the mapped structure contract, ADR and user guide without raising size baselines.

Verification

All executions used ubuntu, Bun 1.4.0, isolated worktrees and fresh private HOME/CODEX_HOME/OPENCODEX_HOME/TMPDIR; no live client/provider invocation.

  • Failure-before: new bun test tests/codex-integration/codex-clamp-notice.test.ts against unchanged source: 4 pass / 2 fail (diagnostic rewrite and repeated warning).
  • Fix-after, same new regression file: 6 pass / 0 fail, 16 assertions.
  • Existing consumers: bun test tests/codex-integration/codex-runtime.test.ts -t clamp: 6 pass / 0 fail, 19 assertions (53 unrelated cases filtered).
  • bun scripts/structure-ssot.ts --fix: passed after splitting the new contract into its mapped document.
  • bun test tests/test-layout.test.ts tests/test-layout-tooling.test.ts tests/ci-workflows/file-size-ratchet.test.ts: 28 pass / 0 fail, 643 assertions.
  • git diff --cached --check: passed before commit.
  • Focused cgroup: CPUQuota=75%, MemoryHigh=1280M, MemoryMax=1536M, MemorySwapMax=0, TasksMax=64; small gates CPU=50%, high=384M/max=512M/tasks=32; nice=10, finite runtime limits, enforcement verified inside the workload.
  • Deferred: whole-project typecheck/full suite/docs build and Windows/macOS qualification. A preceding local full typecheck on the same dev base exceeded the 240-second resource envelope; local whole-project typecheck was not retried for this patch. See the exact-head hosted typecheck receipt below.
  • No manual security/privacy scans. Only the automatic privacy CI of this newly created PR is permitted by the user.

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.

The last item is not self-attested: this patch changes local diagnostic deduplication, not authorization or credentials; any required independent review remains separate. No merge, release or deployment requested.

Exact-head follow-up

Initial automatic CI on 642d0a3fdf failed Typecheck with TS4104: the existing diagnostic type exposes readonly arrays. Commit 5f81eab523 changes only the comparison helper parameter to readonly string[] (no runtime mutation or algorithm change). The focused new regression file was rerun on this final head: 6 pass / 0 fail, 16 assertions. The earlier failure is not presented as a pass. On final head 5f81eab52337221d27b96893d0d742fb4feea551, automatic CI 38270390491 completed its Typecheck step successfully, and its docs-site-build and structure-gate jobs passed. Selected Linux test shards and remaining gates are still running. Windows/macOS full test shards are unselected, not validated. The focused local evidence plus hosted typecheck/docs results support requesting review under the documented full-suite resource exception, not merging or claiming all CI passed.

Summary by CodeRabbit

  • Bug Fixes
    • Sync now avoids repeating clamp notices when the diagnostic is unchanged. Notices are reported again if the clamp clears and later returns.
    • Removed the “Run ocx doctor” hint from clamp notices.
  • Documentation
    • Updated the Codex integration guide with clamp notice behavior and added reference documentation for effort clamping.

@coderabbitai

coderabbitai Bot commented Oct 12, 2026 •

Copy link
Copy Markdown
Contributor

Review in 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: f32a040c-a2bc-4989-bd3b-67f1f3ed900b

📥 Commits

Reviewing files that changed from the base of the PR and between 3e58c2c and 5f81eab.


📒 Files selected for processing (11)
  • docs-site/src/content/docs/guides/codex-integration.md
  • scripts/test-layout/layout.json
  • src/codex/catalog/effort.ts
  • src/codex/runtime.ts
  • structure/INDEX.md
  • structure/catalog-effort-clamp.md
  • structure/catalog.md
  • structure/decisions/ADR-7004-effort-clamp-observation.md
  • structure/manifest.json
  • tests/codex-integration/codex-clamp-notice.test.ts
  • tests/fixtures/test-layout-expected.json

Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 8 remain after this review.



📝 Walkthrough
📝 Walkthrough

Walkthrough

Codex sync now compares effort-clamp observations before persisting them. It emits a clamp notice only when an observation changes, preserves callbacks for each nonempty clamp, and omits the doctor hint. Tests and documentation cover persistence, repeat notices, and clamp recurrence.

Changes

Codex effort-clamp observations

Layer / File(s) Summary
Persist changed clamp observations
src/codex/runtime.ts:228-261, tests/codex-integration/codex-clamp-notice.test.ts:1-44
persistEffortClamp returns false when there is no diagnostic to persist or the existing observation matches. It compares runtime path, version, and deduplicated effort and model sets. Changed observations are written and return true. Tests check unchanged file bytes, inode, and modification time, and persistence when diagnostic fields change.
Emit notices for changed observations
src/codex/catalog/effort.ts:623-624, src/codex/runtime.ts:1235, tests/codex-integration/codex-clamp-notice.test.ts:46-73, scripts/test-layout/layout.json:684, tests/fixtures/test-layout-expected.json:479
Catalog clamping logs only when the observation changed, except command-candidate runs, which count as changed. The callback still runs for each nonempty clamp. The clamp notice no longer suggests running ocx doctor. Tests cover repeated observations and notice recurrence after clearing.
Document clamp observation behavior
docs-site/src/content/docs/guides/codex-integration.md:38-42, structure/catalog-effort-clamp.md:1-22, structure/catalog.md:3, structure/decisions/ADR-7004-effort-clamp-observation.md:1-16, structure/INDEX.md:31,143-144, structure/manifest.json:162-168
The guide, structure documentation, and ADR describe clamp projection, persistence, notice conditions, callbacks, and recurrence. They also document the nonblocking runtime snapshot and integration-test coverage.

Priority: ⬇️ Low

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

Change: Bug fix · Severity of issue fixed: Low

Suggested reviewers: lidge-jun


Merge Risk: ⚪ Minimal · up to 5f81e

The clamp-observation change is mergeable after normal checks; no actionable issue is established.

Pre-merge checks | Passed 4 | Failed 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage Warning Docstring coverage is 20.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 5 functions across 3 files. (8 skipped: 8… Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Linked Issues check Passed Issue [#7004] requires deduplicated clamp notices, stable diagnostic persistence for unchanged observations, repeat reporting after a changed or cleared clamp, and removal of the misleading doctor hin…
Out of Scope Changes check Passed The changed files remain connected to [#7004]. The new regression test and its test-layout mappings support the required behavior. The updates to `docs-site/src/content/docs/guides/codex-integration.m…
Description Check Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check Passed The title clearly and concisely describes the main change: preventing repeated persistence and notices for unchanged Codex effort-clamp observations.

Full details: Docstring Coverage

Explanation

Docstring coverage is 20.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 5 functions across 3 files. (8 skipped: 8 unsupported.)


  • Fix all pre-merge checks with AI
✨ Finishing Touches 💡 1
📝 Generate docstrings 💡
  • Commit to this branch
  • Create a new PR

🧪 Generate unit tests (beta)
  • Commit to this branch
  • Create a new PR

  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

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 github-actions Bot added the bug Something isn't working label Oct 12, 2026
@github-actions

Copy link
Copy Markdown
Contributor

✅ Deterministic PR hygiene checks passed.

@lidge-jun

Copy link
Copy Markdown
Owner

Landed on dev through #7008 (merge a63267c134), carried as commit 419860b with your authorship and a Co-authored-by trailer preserved. Thank you — this ships in 2.84.0.

@lidge-jun lidge-jun closed this Oct 12, 2026
phillipod pushed a commit to phillipod/opencodex that referenced this pull request Oct 12, 2026
…n#7005)

Carried from lidge-jun#7005 into the 2.84.0 release train (lane L2).
Fixes lidge-jun#7004.

Co-authored-by: Ingwannu <ingwannu@users.noreply.github.com>
phillipod pushed a commit to phillipod/opencodex that referenced this pull request Oct 12, 2026
Plan, architect consultation, audit amendments and final review for the
2.84.0 release-train contributor carry (lidge-jun#7005 lidge-jun#7001 lidge-jun#7003 lidge-jun#7002 lidge-jun#6968).
@lidge-jun lidge-jun mentioned this pull request Oct 13, 2026
3 tasks done
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants