Skip to content

docs(directive): sync product-goal-directive.md with the user's current text - #1757

Open
seonghobae wants to merge 14 commits into
mainfrom
docs/product-goal-directive-sync-20260903
Open

docs(directive): sync product-goal-directive.md with the user's current text#1757
seonghobae wants to merge 14 commits into
mainfrom
docs/product-goal-directive-sync-20260903

Conversation

@seonghobae

@seonghobae seonghobae commented Sep 3, 2026

Copy link
Copy Markdown
Contributor

Summary

  • Closes the "loop-prompt-vs-file drift" question two peer sessions independently flagged to the user this cycle: I now have the user's actual current /loop text (pasted directly into this session) and diffed it against docs/product-goal-directive.md section by section.
  • Six of nine sections had genuinely new content missing from the file — appended as dated addenda, preserving every existing quoted section and prior Devin/CodeRabbit correction note untouched.
  • docs/doctoring/product-goal-directive-sync-20260903.md records the full diff table and resolves the open question: the "i18n list" and "core-foundation ownership map" content a peer had briefly mis-cited as being in the wrong section of the file was real content — it just wasn't in the file at all yet.

Test plan

  • Docs-only change; ran the two existing contract tests that assert specific substrings of docs/product-goal-directive.md (tests/test_noema_orchestrator_workflow_contract.py, tests/test_pr_review_autofix_writer_security_contract.py) — both pass, confirming the pinned substrings survived the edit.
  • python3 -m pytest tests/test_noema_orchestrator_workflow_contract.py tests/test_pr_review_autofix_writer_security_contract.py -q → 16 passed.

🤖 Generated with Claude Code


Devin Review

…nt text

Two peer sessions this cycle independently flagged that at least one live
/loop's standing text carried content (an i18n language list, LLM
provider-group/timeout language, a fuller core-foundation ownership map)
that this file did not have, and correctly declined to guess at a fix
themselves. The user then re-pasted their current full /loop invocation
directly into this session, giving primary-source text to reconcile against
instead of a secondhand description.

Diffed all nine sections in full. Six had genuinely new content (the "PR 0"
refinement in #1, a full Close-vs-repair policy in #2, a decision-record
completeness bar in #3, the 8-language i18n list + DB-versioned-translation
architecture in #4, an explicit p95<=20ms target in #7, null-timeout
semantics + admin-web scope in #8, and a much larger categorized
core-foundation ownership map naming 15 repos not previously listed in #9).
Appended each as a dated addendum, preserving all existing quoted text and
prior Devin/CodeRabbit correction notes untouched. Two sections (#5, #6)
had no new content and were left alone.

docs/doctoring/product-goal-directive-sync-20260903.md records the full
diff table and resolves the open question both peers had flagged to the
user: the content they described was real, just genuinely missing from
this file -- not a citation error on either peer's part.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
@coderabbitai

coderabbitai Bot commented Sep 3, 2026

Copy link
Copy Markdown

Warning

Review limit reached

Next included review available in 46 minutes.

Check out review usage here.

View limit details

Limit details: You’ve used the included review currently available.

You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository.

Learn how review limits work.

Review configuration:

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Team

Run ID: 12c97c94-6070-441d-9abf-3d13f842152d

📥 Commits

Reviewing files that changed from the base of the PR and between f2f91b8 and cb7c1f5.

📒 Files selected for processing (2)
  • docs/doctoring/product-goal-directive-sync-20260903.md
  • docs/product-goal-directive.md

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.

devin-ai-integration[bot]

This comment was marked as resolved.

seonghobae and others added 6 commits September 3, 2026 10:26
…Devin Review)

Devin correctly caught that this addendum's editorial gloss ("not a hard
gate that blocks merges on its own") weakened a requirement the user stated
unconditionally ("맞춘다"), and this file's own header explicitly warns
against reinterpreting quoted sections when copying them. Removes the
softening; the addendum now only clarifies measurement method (realistic
benchmarks, no shortcuts), not whether the target binds.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
seonghobae added a commit that referenced this pull request Sep 5, 2026
…works (#1868)

The .github-side follow-up this entry named as still-needed (call_llm not
reading the HTTPError response body, so it couldn't surface served_model)
shipped in #1831 (merged), hardened by #1835 and #1850.

Found incidentally while handling an unrelated Autofix event on PR #1757: a
fresh gateway failure now logs phase=response_error and a real model name
(served_model=google/gemma-4-31b-it) instead of the old unknown/connecting
pair. The underlying gateway instability (502 after 284.7s) is still a
separate, open, recurring problem -- but the telemetry gap that made every
prior instance of it undiagnosable is closed.

Co-authored-by: Claude Sonnet 5 <noreply@anthropic.com>
Co-authored-by: opencode-agent[bot] <219766164+opencode-agent[bot]@users.noreply.github.com>
@seonghobae

Copy link
Copy Markdown
Contributor Author

The single red check here is not caused by this pull request and cannot be fixed from this branch. Recording the evidence so the next person does not spend the diagnosis again, and re-running only the failed job rather than pushing a commit.

What actually failed

noema-review, run 33969604299, step Prepare Noema model verdict, on a runner it held for 35.5 minutes:

"provider": "nvidia_nim"
"provider": "nvidia_nim_sub"      "error_type": "HTTPError"
"provider": "nvidia_nim_sub"      "error_type": "HTTPError"
"provider": "nvidia_nim_sub"

##[error]Noema gateway transport failed: HTTPError: HTTP Error 503: Service Unavailable;
         caller attempts=1, duration=1433.5s, phase=response_error,
         served_model=deepseek-ai/deepseek-v4-flash-0731

Every route the sidecar probed is nvidia_nim or nvidia_nim_subno OpenRouter or Bytez route at all, so the gateway's failover loop had a single upstream family to fail over within. When that family stalled it held the slot for 1433 s and returned 503 to the caller.

This is the org-wide chain already root-caused elsewhere: scripts/ci/contextual_orchestrator_review_launcher.py:129 drops every OpenRouter row before serving —

return [model for model in (discovered or []) if not getattr(model, "evidence_only", False)]

— while the policy layer had admitted free routes across three credential accounts. #1476 is the fix and is still open (mergeable, behind); that line is unchanged on main as of this comment.

Why no commit was pushed here

This PR is documentation-only (docs/product-goal-directive.md). Nothing in its diff reaches the review sidecar. A commit would have:

  • failed the same way, since the launcher defect is still live on main — merging main in does not help either;
  • re-fanned all 22 checks, of which 13 are currently green, forcing them to be re-earned in a queue that is at its 60-job ceiling;
  • burned another multi-minute runner slot on the same 503.

Instead I re-ran only the failed job (rerun-failed-jobs, now attempt 2, queued). The 13 green checks are untouched. A retry can pass if it lands on a healthy NVIDIA endpoint, which is the only lever available from this side until #1476 lands.

If this check goes red again with the same nvidia_nim-only probe list and a 5xx, that is the same structural failure, not a regression in this PR.

🤖 Generated with Claude Code

@seonghobae

Copy link
Copy Markdown
Contributor Author

Correction to my comment above: the cause I named is wrong. The conclusion is unchanged — this failure is not from this PR and no commit here fixes it — but the mechanism is not the launcher's evidence_only filter. I verified the real one from source before writing this.

scripts/ci/contextual_orchestrator_review_launcher.py:129 does drop evidence_only rows, but on the pinned contextual-orchestrator revision OpenRouter rows carry evidence_only=False and pass through it. That line is a hardening concern, not what starved this run.

What actually starves it — an account cap the sidecar raises above the policy default.

scripts/ci/contextual_orchestrator_review_sidecar.sh:38   CATALOG_LIMIT="${ORCHESTRATOR_CATALOG_LIMIT:-12}"
scripts/ci/contextual_orchestrator_review_sidecar.sh:43   CATALOG_ACCOUNT_CAP="${ORCHESTRATOR_CATALOG_ACCOUNT_CAP:-8}"

scripts/ci/contextual_orchestrator_review_policy.py:34    DEFAULT_CATALOG_LIMIT = 12
scripts/ci/contextual_orchestrator_review_policy.py:35    DEFAULT_ACCOUNT_CAP = 4

build_zdr_prioritized_catalog sorts by (cost_rank, zdr_rank, provider, model) — provider is the third key, so ties break alphabetically — and then fills:

for row in eligible_rows:
    account = provider_account(str(row["provider"]))
    if per_account[account] >= account_cap:   # 8, from the sidecar
        continue
    per_account[account] += 1
    picked.append(row)
    if len(picked) >= limit:                  # 12
        break

nvidia_nim < nvidia_nim_sub < openrouter alphabetically. With account_cap = 8: nvidia_nim takes 8 and hits its cap, nvidia_nim_sub takes 4, len(picked) reaches the 12 limit, and the loop breaks before OpenRouter is ever examined. With the policy's own default of 4 it would be 4 + 4 + 4 — three accounts, same 12 routes.

That is the 8 + 4 in the log quoted above, and why the failover loop had one upstream family to fail over within before returning 503 after 1433 s.

Credit: jan lane found and corrected this after host1 refuted the launcher explanation from source; I re-derived the cap/sort interaction here rather than relaying it.

What does not change: this PR is documentation-only, nothing in its diff reaches the sidecar, and merging main in does not help — the cap line is unchanged on main. Re-running only the failed job (attempt 2) was still the right lever, and the 13 green checks are still untouched. #1476 is not the fix for this; the fix is a change to the sidecar's cap default plus round-robin fill across accounts within a tier, which the jan lane has claimed.

🤖 Generated with Claude Code

New review runs bind workflow_sha at creation time, so the 13:40:58Z runs that
failed executed the pre-#1939 sidecar catalog. This merge creates a fresh event
so the current round-robin catalog is used, and clears mergeable_state=behind.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant