Fix citation loss, effort clamping and nested inferenceRequest for PR #7219

Three review findings, each with a regression test that fails without the fix.

Citation dropped for a bare URL in prose parentheses. _RAW_URL swallows the
closing paren and the old trim set only stripped ".,;:!?", so the catalog
lookup missed and the validator deleted the whole citation, leaving an
unbalanced "(" in the report. New _trim_url_tail follows GFM extended autolink
path validation: one right-to-left pass that interleaves punctuation and
unmatched-")" trimming. Both rules must run in the same loop, else
"https://x/y.)" keeps a stray dot. Balanced parens inside a URL
(Wikipedia-style) still survive. Output verified against cmark-gfm on nine
cases, including "https://x/foo)bar)" which must keep ")bar".

Research runs forwarded reasoningEffort unclamped. The local chat path clamps
to the loaded model's advertised levels; the research branch did not, and the
backend only validates enum membership, so llama.cpp dropped a level the model
lacks and the whole durable run silently fell back to the template default.
Now uses the same helper and the same levels as normal chat. Note this makes
"max" on a gpt-oss low|medium|high model resolve to "low" rather than falling
through to the template default, matching normal chat exactly; the divergence
between the two paths was the bug.

Nested inferenceRequest values were persisted. Every allowed field is a scalar
and the numeric/bool/enum ones reject a container while coercing, but "model"
is stringified with str(), which never raises, so {"auth": "sk-..."} slipped
past the sensitive-key scan ("auth" is not on the list) into the durable run
config as the model id. Mirrors the ragScope guard already in this PR.

Verified: 542 passed across the research/web/sandbox/chat-history backend
suites, frontend contract 10 passed, tsc --noEmit clean.
This commit is contained in:
danielhanchen 2026-07-26 12:14:57 +00:00
commit c715df1cf4
5 changed files with 99 additions and 3 deletions

View file

@ -645,6 +645,29 @@ def _split_rag_result(result: str) -> tuple[str, list[dict[str, Any]]]:
return text.rstrip(), sources
def _trim_url_tail(raw: str) -> str:
"""Strip trailing prose punctuation that ``_RAW_URL`` swallowed.
Mirrors GFM extended autolink path validation: walk right to left, dropping
``.,;:!?`` and any ``)`` that has no matching ``(`` inside the URL, stopping at the
first character that is neither. Both rules must run in one interleaved pass, else
``https://x/y.)`` keeps a stray dot. Without this, ``(https://x/y)`` never matches
the catalog and the citation is dropped from the report.
"""
end = len(raw)
opening, closing = raw.count("("), raw.count(")")
while end:
char = raw[end - 1]
if char == ")":
if closing <= opening:
break
closing -= 1
elif char not in ".,;:!?":
break
end -= 1
return raw[:end]
def _research_step_failed(web_result: str, rag_sources: list[dict]) -> bool:
return is_tool_error(web_result) and not rag_sources
@ -746,10 +769,11 @@ def _validate_report_sources(report: str, sources: list[dict]) -> str:
def replace_raw_url(match: re.Match) -> str:
# Cite whole source URLs; drop other raw URLs. Whole-match avoids prefix collisions.
raw = match.group(0)
core = raw.rstrip(".,;:!?")
core = _trim_url_tail(raw)
if core in source_by_url:
return (citation(core) or core) + raw[len(core) :]
return ""
# Keep the trimmed tail so dropping the URL cannot unbalance the prose.
return raw[len(core) :]
validated = replace_markdown_links(report)
validated = _AUTOLINK.sub(replace_autolink, validated)

View file

@ -183,6 +183,11 @@ def _sanitize_config(payload: CreateResearchRun, thread: dict) -> dict:
status_code = 400,
detail = f"Unsupported inferenceRequest fields: {', '.join(sorted(unknown))}",
)
# Mirrors the ragScope guard below. Every allowed field is a scalar; the numeric/bool/enum ones
# reject a container while coercing, but "model" is stringified, so {"auth": "sk-..."} slips past
# the sensitive-key scan (inner key unlisted) into the durable config as the model id.
if any(isinstance(value, (dict, list, tuple)) for value in request.values()):
raise HTTPException(status_code = 400, detail = "Invalid inferenceRequest value")
model = str(request.get("model") or thread.get("modelId") or "").strip()
if not model:
raise HTTPException(status_code = 400, detail = "A selected local model is required")

View file

@ -133,6 +133,23 @@ def test_sanitize_config_rejects_nested_inference_credential():
_sanitize_config(payload, {"modelId": "m"})
def test_sanitize_config_rejects_nonscalar_inference_request_value():
# Companion to the ragScope case below. "model" is the one allowed field coerced with str(),
# which never raises, so a container whose inner key is not on the sensitive list ("auth" is
# not) was stringified into the durable run config as the model id.
for request in ({"model": {"auth": "sk-private-value"}}, {"model": ["sk-private-value"]}):
with pytest.raises(Exception):
_sanitize_config(_make_payload(inferenceRequest = request), {"modelId": "m"})
def test_sanitize_config_accepts_scalar_inference_request():
# Well-formed runs must be unaffected by the rejection above.
request = {"model": "m", "temperature": 0.7, "topP": 0.9,
"maxTokens": 1024, "enableThinking": True, "reasoningEffort": "high"}
config = _sanitize_config(_make_payload(inferenceRequest = dict(request)), {"modelId": "other"})
assert config["inferenceRequest"] == request
def test_sanitize_config_rejects_nested_rag_scope_secret():
payload = _make_payload(ragScope = {"kb_id": {"token": "rag-secret"}})
with pytest.raises(Exception):
@ -199,3 +216,33 @@ def test_raw_url_citation_does_not_collide_on_prefix():
)
assert "[Report](https://ex.com/report)" in out
assert "/report)-attack" not in out
def test_raw_url_in_prose_parentheses_keeps_its_citation():
# ``_RAW_URL`` swallows the closing paren, so the catalog lookup used to miss and the
# whole citation was deleted, leaving an unbalanced "(" in the report.
sources = [{"url": "https://ex.com/report", "title": "Report"}]
out = _validate_report_sources("Public (https://ex.com/report) today.", sources)
assert out == "Public ([Report](https://ex.com/report)) today."
def test_raw_url_keeps_parentheses_that_belong_to_the_url():
# Only unmatched trailing parens are prose; Wikipedia-style URLs must survive both bare
# and wrapped (GFM extended autolink path validation).
url = "https://en.wikipedia.org/wiki/Mercury_(planet)"
sources = [{"url": url, "title": "Mercury"}]
assert f"[Mercury]({url})" in _validate_report_sources(f"Bare {url} ok.", sources)
assert f"[Mercury]({url})" in _validate_report_sources(f"Wrapped ({url}) ok.", sources)
def test_raw_url_trailing_punctuation_is_trimmed_in_one_pass():
# Trimming parens and punctuation in separate passes leaves a stray "." on ".)"; both
# rules have to run right to left in the same loop.
sources = [{"url": "https://ex.com/x", "title": "X"}]
assert "[X](https://ex.com/x)." in _validate_report_sources("End (https://ex.com/x.).", sources)
def test_dropped_raw_url_does_not_unbalance_prose():
# An uncataloged URL is still removed, but the paren it swallowed belongs to the prose.
out = _validate_report_sources("Claim (https://nope.com/x) here.", [])
assert out == "Claim () here."

View file

@ -2170,7 +2170,13 @@ export function createOpenAIStreamAdapter(
(runtime.reasoningStyle === "reasoning_effort" ||
runtime.reasoningStyle === "enable_thinking_effort")
) {
inferenceRequest.reasoningEffort = runtime.reasoningEffort;
// Clamp like normal chat does. reasoningEffort is one shared persisted setting and
// the load paths refresh reasoningEffortLevels without re-clamping it, so a level
// this model lacks is dropped by llama.cpp and the run falls back to the default.
inferenceRequest.reasoningEffort = clampReasoningEffortToLevels(
runtime.reasoningEffort,
runtime.reasoningEffortLevels,
);
}
const researchProjectId = await resolveProjectId(resolvedThreadId);
const projectRagEnabled = researchProjectId

View file

@ -83,6 +83,20 @@ def test_research_mode_is_single_chat_and_detaches_without_cancel() -> None:
assert "resolveChatInstructions" in adapter
def test_research_reasoning_effort_is_clamped_to_the_loaded_model() -> None:
# A level the loaded model lacks is dropped by llama.cpp, so the whole durable run
# would silently fall back to the template default. Must use the same helper and the
# same levels as normal local chat so the two paths cannot drift apart again.
adapter = source("features/chat/api/chat-adapter.ts")
branch = adapter.split("Deep research requires a selected local model.", 1)[1].split(
"createdRun = await createResearchRun({", 1
)[0]
assert "inferenceRequest.reasoningEffort = runtime.reasoningEffort;" not in branch
assert "inferenceRequest.reasoningEffort = clampReasoningEffortToLevels(" in branch
assert "runtime.reasoningEffortLevels," in branch
assert "const localReasoningEffort = clampReasoningEffortToLevels(" in adapter
def test_research_metadata_and_server_merge_are_persisted() -> None:
adapter = source("features/chat/api/chat-adapter.ts")
runtime = source("features/chat/runtime-provider.tsx")