Skip to content

fix(chat_model): handle plain Azure-OpenAI keys without JSONDecodeError (#17204) - #17215

Open
Harsh23Kashyap wants to merge 1223 commits into
infiniflow:mainfrom
Harsh23Kashyap:fix/chat-model-azure-json-decode-fallback
Open

fix(chat_model): handle plain Azure-OpenAI keys without JSONDecodeError (#17204)#17215
Harsh23Kashyap wants to merge 1223 commits into
infiniflow:mainfrom
Harsh23Kashyap:fix/chat-model-azure-json-decode-fallback

Conversation

@Harsh23Kashyap

Copy link
Copy Markdown
Contributor

No description provided.

xugangqiang and others added 30 commits July 10, 2026 22:47
### Summary

Refine ingestion task state transitions
…#16794)

## What

An **Await Response** (`UserFillUp`) node placed inside a **Loop** now
pauses and waits for a fresh user response on **every** iteration,
instead of only on the first one.

## Problem

When a `UserFillUp` node lives inside a `Loop`, it only paused for input
on the first iteration. On subsequent iterations the loop ran straight
through, silently reusing the answer the user gave the first time.

Root cause is in `UserFillUp._invoke` / the canvas wait-check
(`agent/canvas.py`). The wait-check decides whether to pause by calling
`Canvas._is_input_field_satisfied` on the node's form fields — a field
counts as satisfied as soon as its `value` is not `None`:

```python
@staticmethod
def _is_input_field_satisfied(field):
    ...
    if value is None:
        return False
    return True
```

The same component object is reused across loop iterations, and
`UserFillUp._invoke` writes the answer into
`self._param.inputs[...]["value"]` via `set_input_value`. Nothing
cleared those values when the node was re-entered for the next
iteration, so:

| Iteration | Entry (no answer yet) | Field value | Satisfied? | Result
|
|---|---|---|---|---|
| 1 | fresh | `None` | no | pauses ✅ |
| 1 | resume w/ answer | `answer` | yes | continues ✅ |
| 2 | fresh | `answer` (**stale**) | yes | continues ❌ (should pause) |

## Fix

When a `UserFillUp` is entered without a fresh user answer
(`merged_inputs` is empty), clear the retained form values so the
wait-check treats the form as unsatisfied and pauses again:

```python
merged_inputs = self._merge_runtime_inputs(kwargs.get("inputs", {}))
if not merged_inputs:
    self._clear_form_values()
```

- Fresh entry / new loop iteration → no answer supplied → values cleared
→ node pauses and waits.
- Resume with an answer → `merged_inputs` is non-empty → values applied
normally, nothing cleared.
- Non-loop behavior is unchanged: the first entry already had `None`
values, so clearing is a no-op there.

`Begin` overrides `_invoke` and is unaffected.

## Tests

Added to
`test/testcases/test_web_api/test_canvas_app/test_fillup_unit.py`:

- `test_user_fillup_clears_stale_values_on_reentry_without_answer` — a
retained value is cleared on a fresh entry with no answer (loop
re-entry).
- `test_user_fillup_keeps_values_when_answer_supplied` — a supplied
answer is applied and not cleared.

All unit tests pass and `ruff check` is clean.

## Scope

This targets the Python agent runtime (`agent/`). It is independent of
any other in-flight Await Response change.
### Summary

Sync code from EE

Signed-off-by: Jin Hai <haijin.chn@gmail.com>
…reprocessing (infiniflow#7316) (infiniflow#16785)

Fixes infiniflow#7316.

## Problem

`deepdoc/vision/operators.py` defines the image-standardize
preprocessing op as `class StandardizeImag` (missing the final `e`), but
every caller — including
`deepdoc/vision/recognizer.py::Recognizer.preprocess` — looks the class
up by the canonical string `"StandardizeImage"` via:

```python
op_type = new_op_info.pop("type")  # "StandardizeImage"
preprocess_ops.append(getattr(operators, op_type)(**new_op_info))
```

So `getattr(operators, "StandardizeImage")` raised `AttributeError`, and
the "StandardizeImage" preprocessing step silently never ran for any
image pipeline that used the dynamic dispatch (LayoutLMv3 and friends).
The user-visible symptom is that the standardize step is missing
entirely from the preprocessing chain, so the model gets un-normalized
images.

## Production fix

```diff
-class StandardizeImag:
+class StandardizeImage:
     """normalize image
     Args:
         mean (list): im - mean
         std (list): im / std
         is_scale (bool): whether need im / 255
         norm_type (str): type in ['mean_std', 'none']
     """
```

That's the entire production change — a one-character class rename. The
misnamed `StandardizeImag` had no other references in the codebase
(verified via `git grep`), so removing it is safe; every caller uses the
canonical `"StandardizeImage"` string and will now resolve correctly.

## Tests

New `test/unit_test/deepdoc/vision/test_operators_standardize_image.py`
with six regression tests, all green locally:

```
test_standardize_image_class_resolves_by_canonical_name            PASSED
test_standardize_image_callable_matches_legacy_alias_name          PASSED
test_standardize_image_normalizes_input_with_mean_std_and_is_scale PASSED
test_standardize_image_skips_scaling_when_is_scale_false           PASSED
test_standardize_image_norm_type_none_passes_image_through         PASSED
test_standardize_image_via_module_getattr_dispatch_path            PASSED
6 passed in 0.18s
```

The tests:
1. **Pin the dispatch contract** (`hasattr(operators,
"StandardizeImage")`) — this is the exact check the recognizer's
`getattr` would do, so any future regression fails the same way the
runtime would.
2. **Pin that the misspelled name is gone** — if a downstream caller
ever relied on it, this fails loudly.
3–5. **Behavioural coverage** of the three documented code paths:
`is_scale=True, norm_type="mean_std"`, `is_scale=False,
norm_type="mean_std"`, and `norm_type="none"`.
6. **End-to-end via the same `getattr(operators, "StandardizeImage")`
call** the recognizer uses, with a real numpy image, so any rename or
removal surfaces as `AttributeError` instead of silently skipping the
step.

Verified both ways:
- Without the fix → **all 6 tests fail** (Python even suggests
`'StandardizeImag' → 'StandardizeImage'`)
- With the fix → all 6 pass in 0.15s

The test file follows the project's existing pattern
(`test/unit_test/deepdoc/parser/test_html_parser.py`): load the target
module via `importlib.util.spec_from_file_location`, stub the only
project-internal import (`rag.utils.lazy_image`), and assert against the
loaded module — no full RAGFlow runtime required.

## Risk

Very low. The class is renamed; no public Python API was using the
misnamed class. The only reference path is the `"StandardizeImage"`
string in `recognizer.py:270`, which now resolves correctly.

## Out of scope

- No other ops in `operators.py` are affected; checked all the others
(DecodeImage, NormalizeImage, Permute, etc.) and they all use correct
names.
- The dynamic-dispatch lookups in `recognizer.py` for `LinearResize`,
`StandardizeImage`, `Permute`, `PadStride` all use the same dispatch
path; only the `StandardizeImage` key was broken. No other keys need
fixing.

Made with [Cursor](https://cursor.com)

---------

Co-authored-by: Taranum01 <Taranum01@users.noreply.github.com>
Co-authored-by: Cursor <cursoragent@cursor.com>
Co-authored-by: Zhichang Yu <yuzhichang@gmail.com>
### What problem does this PR solve?

Issue [infiniflow#16758](infiniflow#16758) —
clicking a chunk whose data references a single-line variable from an
Await-Response (UserFillUp) component, the Agent's `user_prompt` is
being resolved against the **previous** canvas run's captured value
instead of the current run's value. The system-prompt path works only
because the system prompt is computed upstream and re-reads the value on
the new run.

### Root cause

`Canvas._run_impl` reset every path component with `only_output=True`,
so `_param.inputs` was never cleared between runs.
`ComponentBase.get_input()` calls `set_input_value(var, resolved)` at
line 482, which writes the resolved variable into
`self._param.inputs[var]["value"]`. On the next canvas run, that input
was never cleared, so the previous run's resolved value stuck around.
The Agent's `kwargs.get("user_prompt")` then read the stale string and
forwarded it to the LLM, which produced the "Understood. Please provide
the text..." fallback because the prompt looked empty.

### What changed?

- `agent/canvas.py` — differentiate `begin` (still `only_output=True`,
since it has no inputs and the webhook payload branch below populates
`request` explicitly) from non-begin path components (reset with
`only_output=False`, which clears both `inputs` and `outputs`).
- `test/unit_test/agent/test_canvas_input_reset.py` — new pytest module.
Pinned the contract: non-begin path components receive
`only_output=False`. The fix is small enough to verify with a stub
canvas rather than a full canvas-runtime test (the existing agent
conftest hits an unrelated `scholarly` import on Python 3.13, so a real
canvas import would require fixing that first).

### Backward compatibility

- `Begin` behaviour unchanged.
- All non-begin path components: previously persisted inputs across runs
(the bug); now reset between runs. Components that were relying on stale
inputs (none found in the existing test suite) would lose that as a side
effect, but that is the entire point of the fix.
- No API surface change. No backend change.

### Testing

```
$ uv run pytest test/unit_test/agent/test_canvas_input_reset.py -v
collected 4 items
test/unit_test/agent/test_canvas_input_reset.py::test_begin_is_reset_with_only_output_true PASSED
test/unit_test/agent/test_canvas_input_reset.py::test_non_begin_path_components_are_reset_with_only_output_false PASSED
test/unit_test/agent/test_canvas_input_reset.py::test_only_path_components_are_reset PASSED
test/unit_test/agent/test_canvas_input_reset.py::test_inputs_reset_flag_is_passed_to_non_begin_components PASSED
4 passed in 0.14s
```

`python3 -m py_compile agent/canvas.py` clean. Existing agent test files
(`test_switch.py`, `test_llm_prompt.py`) hit a pre-existing `scholarly`
import error on Python 3.13 (unrelated to this PR), so I couldn't run
the full agent suite. Recommend fixing the `scholarly` import
separately.

### Files changed

- `agent/canvas.py` (+9 / −1)
- `test/unit_test/agent/test_canvas_input_reset.py` (new, +104)

Fixes infiniflow#16758

---------

Co-authored-by: Harsh Kashyap <harshkashyap@Harshs-MacBook-Pro.local>
Co-authored-by: Zhichang Yu <yuzhichang@gmail.com>
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
## Summary

- Merge upstream main and retain PubMed component support.
- Preserve newly registered tool components and update registry
verification.

## Tests

- `bash build.sh --test ./internal/agent/component/...`
- `bash build.sh --test ./internal/agent/tool/...`

<img width="1817" height="972" alt="image"
src="https://github.com/user-attachments/assets/9fcb9448-9e26-41b9-940c-a9bfde9835e9"
/>

---------

Co-authored-by: Jin Hai <haijin.chn@gmail.com>
### Summary
1. update docker compose file to start NATS healthy
2. Add two commands
```
RAGFlow(admin)> live;
SUCCESS
RAGFlow(admin)> health;
+---------------+-------+
| field         | value |
+---------------+-------+
| storage       | ok    |
| message_queue | ok    |
| status        | ok    |
| db            | ok    |
| redis         | ok    |
| doc_engine    | ok    |
+---------------+-------+
```

---------

Signed-off-by: Jin Hai <haijin.chn@gmail.com>
As title.

Signed-off-by: Jin Hai <haijin.chn@gmail.com>
…iflow#16808)

## Summary
- register the Go `ArXiv` canvas component and add its input form
- align the Go ArXiv request/schema with Python by keeping only `query`
in runtime args and moving `top_n`/`sort_by` to node params
- keep ArXiv results consistent for canvas output and tool response
handling

## Test
- `bash build.sh --test ./internal/agent/tool
./internal/agent/component`

<img width="1817" height="972" alt="image"
src="https://github.com/user-attachments/assets/7f726dfa-a996-4561-b481-cb0b44bec81c"
/>
…very (infiniflow#16826)

### Summary

1. refactor dataflow_service.go 
2. guard nats message re-delivery
3. support document parse cancelling & re-run
…w#16822)

### Summary

Implement builtin chunk mehtod as ingestion pipeline in GO
…nfiniflow#16851)

### Summary

Feat: Added support for session graph and session essence templates.
### Summary

as title.

---------

Signed-off-by: Jin Hai <haijin.chn@gmail.com>
…16849)

## Summary

- Add the GitHub Canvas component with tool registration and reference
propagation.
- Align the Invoke component with the Python contract for node config,
input form, response output, and timing fields.
- GitHub search and HTTP Invoke now work correctly in the Go Canvas
runtime.

## Tests

- `bash build.sh --test ./internal/agent/tool/...`
- `bash build.sh --test ./internal/agent/component/...`

Note: the untracked go_ragflow_cli file is not part of the PR changes.

<img width="1813" height="1102" alt="image"
src="https://github.com/user-attachments/assets/f69cef32-59a0-4287-a06b-6843d85198cf"
/>


<img width="1813" height="1102" alt="image"
src="https://github.com/user-attachments/assets/b37dfc31-bc9b-4937-a38e-d2184bb157fe"
/>
…nfiniflow#16692)

### Summary

Port the **QWeather** agent tool to the modern `ToolBase` / `_invoke`
interface. It was still written against the removed legacy
`ComponentBase` / `_run` / `be_output` API, so it was non-functional as
an Agent tool — adding it to an Agent raised `AttributeError` because it
had no `get_meta()`. This is the same defect that was fixed for the
AkShare tool in infiniflow#16417.

**Changes**
- `QWeatherParam` now extends `ToolParamBase` with a `meta` exposing a
`query` (location) parameter, and adds `get_input_form()`. Existing
config (`web_apikey`, `lang`, `type`, `user_type`, `time_period`) is
preserved.
- `QWeather` now extends `ToolBase` and implements `_invoke(**kwargs)`
with the standard retry loop, cancellation checks,
`set_output("formalized_content", ...)`, and `thoughts()`. The weather /
indices / air-quality branches and the API error-code messages are kept.
- Added `test/unit_test/agent/component/test_qweather.py` covering the
restored `meta`, param validation, the weather-now and multi-day and
indices branches, the empty-query short-circuit, and the location-lookup
error message.

**Testing**
- `ruff check agent/tools/qweather.py
test/unit_test/agent/component/test_qweather.py` — clean
- `ruff format --check` — clean
- `pytest test/unit_test/agent/component/test_qweather.py`
### Summary

1. refactor message processing
2. delete un-used componentIndexMap
3. unfold (delete) internal/ingestion/task/task_handler.go
### Summary

certain tests fail because of test drift and were fixed, other because
of go issues

---------

Co-authored-by: Wang Qi <wangq8@outlook.com>
…on (infiniflow#16854)

## Summary

- Align Go WenCai and SearXNG behavior, schemas, and node parameters
with Python.
- Add the `WenCai` and `SearXNG` Canvas components and register their
tool factories.
- Match Python's current WenCai behavior by returning an empty report
while its upstream request is disabled.
- Add SearXNG request validation, SSRF-safe DNS pinning, raw result
preservation, and reference rendering.
- Support context cancellation, error envelopes, and lock-safe retrieval
references.

  ## Tests

  Passed:

  - `bash build.sh --test ./internal/agent/tool/...`
  - `bash build.sh --test ./internal/agent/component/...`
  - `bash build.sh --test ./internal/agent/runtime/...`
  - `bash build.sh --test ./internal/agent/...`
  - `cd web && npm run type-check`
  
  
<img width="1900" height="1102" alt="image"
src="https://github.com/user-attachments/assets/ec77d217-d9fd-455a-96ec-9aabf6841109"
/>
  
<img width="1900" height="1102" alt="image"
src="https://github.com/user-attachments/assets/52ac129f-cb65-453d-ae48-cc518803ac23"
/>
### Summary

As title
Unable to test it since I don't have apiKey for `openai` , `Anthropic`
and `Gemini`

---

<img width="1130" height="557" alt="image"
src="https://github.com/user-attachments/assets/11570c75-68f3-490d-8186-4ecbcd8b8f40"
/>
### Summary

```
RAGFlow(admin)> show version;
+--------------+-----------------------+
| field        | value                 |
+--------------+-----------------------+
| version      | v0.26.4-84-g547bc8614 |
| version_type | open source           |
+--------------+-----------------------+
```

---------

Signed-off-by: Jin Hai <haijin.chn@gmail.com>
…ate is displayed at the end. (infiniflow#16861)

### Summary

Feat: If the interval between two outputs exceeds 600ms, a loading state
is displayed at the end.
### Summary

In Go and python implementation, the dataset / KB id isn't validated if
it is accessible by this user.

---------

Signed-off-by: Jin Hai <haijin.chn@gmail.com>
…niflow#16758) (infiniflow#16792)

## Summary

`ComponentBase.variable_ref_patt` (and its duplicate in
`agent.canvas.Graph.get_value_with_variable`) is the regex the canvas
runtime uses to find `cpn_id@var_nm` template refs in component prompts.

The `cpn_id` half was constrained to `[a-zA-Z:0-9]+`, which silently
dropped underscores. Component ids emitted by the frontend all contain
underscores (`userfillup_abc`, `retrieval_xyz`, `llm_0`, `message_0`,
…), so any template ref like `{userfillup_abc@line}` failed to match.
The placeholder then leaked through to the LLM verbatim, and the Agent
answered only its system-prompt directive.

This is exactly the "unconsidered await response" symptom in infiniflow#16758:

```
Begin(Task) -> Await response -> Agent -> Message
```

Widen `cpn_id` from `[a-zA-Z:0-9]+` to `[a-zA-Z0-9_]+`. Bare `{line}`
(no cpn_id) remains unrecognised so it stays literal until the user
wires it up — matching the existing `VARIABLE_REF_PATTERN` shape used by
`agent.dsl_migration` for the same purpose.

## Changes

- `agent/component/base.py` — fix `variable_ref_patt` class attribute.
- `agent/canvas.py` — same fix applied to the inline regex inside
`Graph.get_value_with_variable` (kept as the literal regex to avoid
coupling the two unrelated sites).
-
`test/testcases/test_web_api/test_canvas_app/test_variable_ref_pattern_unit.py`
— new regression test pinning both the regex shape and end-to-end
resolution.

## Regression coverage

```
test_variable_ref_patt_matches_underscored_component_ids     PASSED
test_variable_ref_patt_still_matches_legacy_ids              PASSED
test_get_input_elements_from_text_resolves_underscored_id    PASSED
test_string_format_substitutes_underscored_ref                PASSED
test_variable_ref_patt_does_not_match_bare_var_name          PASSED
```

All five regression tests fail against the pre-fix regex (verified via
`git stash` round trip — drop fix, tests fail, restore fix, tests pass).

The two targeted existing tests in the same directory
(`test_fillup_unit.py`, `test_iterationitem_unit.py`) continue to pass.

## Repro before the fix

```python
import re
patt = r"\{* *\{([a-zA-Z:0-9]+@[A-Za-z0-9_.-]+|sys\.[A-Za-z0-9_.]+|env\.[A-Za-z0-9_.]+)\} *\}*"
list(re.finditer(patt, "{userfillup_abc@line}"))
# => []   # <-- bug
```

## Repro after the fix

```python
import re
patt = r"\{* *\{([a-zA-Z0-9_]+@[A-Za-z0-9_.-]+|sys\.[A-Za-z0-9_.]+|env\.[A-Za-z0-9_.]+)\} *\}*"
list(re.finditer(patt, "{userfillup_abc@line}"))
# => [<re.Match object; span=(0, 24), match='{userfillup_abc@line}'>]
```

Fixes infiniflow#16758

## Test plan

- [x] New unit tests pass
- [x] Reverse-apply the fix and confirm the regression tests fail (they
do)
- [x] `test_fillup_unit.py` (existing sibling suite) still passes
- [x] `test_iterationitem_unit.py` (existing sibling suite) still passes
- [ ] Project CI green

---------

Co-authored-by: Taranum01 <taranum01@users.noreply.github.com>
Co-authored-by: Cursor <cursoragent@cursor.com>
@dosubot dosubot Bot added 🐞 bug Something isn't working, pull request that fix bug. 🧪 test Pull requests that update test cases. labels Jul 22, 2026
@coderabbitai

coderabbitai Bot commented Jul 22, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: a5a735bf-6563-459a-90c4-14ef7a267564

📥 Commits

Reviewing files that changed from the base of the PR and between eade070 and 051202a.

📒 Files selected for processing (1)
  • test/unit_test/rag/llm/test_chat_model_azure_key_fallback.py
🚧 Files skipped from review as they are similar to previous changes (1)
  • test/unit_test/rag/llm/test_chat_model_azure_key_fallback.py

📝 Walkthrough

Walkthrough

Azure credential parsing is centralized in rag.llm.key_utils and reused by chat, CV, and embedding providers. Tests cover JSON credentials, plain keys, malformed inputs, missing fields, and Azure chat initialization.

Changes

Azure credential resolution

Layer / File(s) Summary
Shared Azure credential resolver
rag/llm/key_utils.py
Adds shared JSON parsing, default API version handling, warnings, raw-key fallback, and module export wiring.
Provider integration
rag/llm/chat_model.py, rag/llm/cv_model.py, rag/llm/embedding_model.py
Uses the shared resolver for Azure chat, CV, and embedding initialization, removing duplicated local implementations.
Regression coverage
test/unit_test/rag/llm/test_chat_model_azure_key_fallback.py
Tests credential parsing and Azure chat construction for plain, JSON, incomplete, array, and invalid inputs.

Estimated code review effort: 3 (Moderate) | ~20 minutes

Poem

I’m a small rabbit with keys in my den,
JSON or plain, they work again.
API versions neatly aligned,
Shared little helpers, neatly designed.
Hop, hop—tests guard the way!

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Description check ⚠️ Warning No author-provided description was added, so the required Summary section and background context are missing. Add a ### Summary section describing the problem, the fix, and any relevant background context for reviewers.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly matches the main change: handling plain Azure-OpenAI keys without JSONDecodeError.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
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.

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.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🧹 Nitpick comments (1)
rag/llm/key_utils.py (1)

55-55: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Keep the resolver internal.

All current consumers explicitly import _resolve_azure_credentials, so adding this underscore-prefixed helper to __all__ unnecessarily creates a public compatibility surface.

Proposed fix
-__all__ = ["_normalize_replicate_key", "_resolve_azure_credentials"]
+__all__ = ["_normalize_replicate_key"]

As per coding guidelines, “Reduce public surface area by making helpers private or internal when possible.”

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@rag/llm/key_utils.py` at line 55, Remove _resolve_azure_credentials from
__all__ in key_utils.py, leaving only the intended public export while keeping
the helper itself and its existing internal consumers unchanged.

Source: Coding guidelines

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@test/unit_test/rag/llm/test_chat_model_azure_key_fallback.py`:
- Around line 123-125: In the test setup around SupportedLiteLLMProvider, stop
assigning the unnecessary MiniMax enum member and scope the OpenRouter
assignment to the existing context manager so it is automatically restored after
the test. Ensure the shim cannot leak state into subsequent tests, preferably by
deleting or otherwise resetting the patched attribute during teardown.

---

Nitpick comments:
In `@rag/llm/key_utils.py`:
- Line 55: Remove _resolve_azure_credentials from __all__ in key_utils.py,
leaving only the intended public export while keeping the helper itself and its
existing internal consumers unchanged.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 6652bbd4-847a-4166-81f6-71f740f4c8bf

📥 Commits

Reviewing files that changed from the base of the PR and between b175ed5 and eade070.

📒 Files selected for processing (5)
  • rag/llm/chat_model.py
  • rag/llm/cv_model.py
  • rag/llm/embedding_model.py
  • rag/llm/key_utils.py
  • test/unit_test/rag/llm/test_chat_model_azure_key_fallback.py

Comment thread test/unit_test/rag/llm/test_chat_model_azure_key_fallback.py Outdated
Move the SupportedLiteLLMProvider.OpenRouter assignment inside the
with block using patch.object(..., create=True) so the patched member
is automatically restored after the test and cannot leak into
subsequent tests. Drop the unused MiniMax assignment — the Azure
branch in LiteLLMBase.__init__ never reaches the MiniMax lookup.
This was referenced Jul 28, 2026
Harsh23Kashyap added a commit to Harsh23Kashyap/ragflow that referenced this pull request Aug 20, 2026
Pre-fix, the Azure-OpenAI provider in rag/llm had 4 unfixed call sites
with 3 different patterns for parsing the key:

  - chat (chat_model.py:1649-1651): bare json.loads(key).get(...) with
    no try/except; crashes with JSONDecodeError on a plain Portal API
    key (the most common user mistake) and AttributeError on any JSON
    non-object input.
  - vision / CV (cv_model.py:380): local helper with silent fallback
    for non-object JSON, silently using the raw key string.
  - embed (embedding_model.py:325): identical local helper, a duplicate
    copy of the CV one.
  - seq2txt (sequence2txt_model.py:386): raw key passed straight to
    AzureOpenAI; a JSON string was used as the api_key and the call
    silently failed at the API with a 401.

Unify all 4 through a single _resolve_azure_credentials helper in
rag/llm/key_utils.py that follows the same pattern as the other 5
helpers in the JSON-decode family (Bedrock, BaiduYiyan, VolcEngine,
OpenRouter, GoogleCV):

  1. Accepts a dict (returned verbatim) or a JSON-string-encoded dict.
  2. On non-JSON input, raises a clear ModelException (retryable=False)
     naming the required fields and pointing at conf/models/azure.json,
     instead of letting json.loads bubble up as JSONDecodeError.
  3. On a JSON top-level type that is not a dict (list, string,
     number, bool, null), raises the same clear ModelException instead
     of calling .get('api_key') on the value and getting AttributeError.

Returns (api_key, api_version) where api_version defaults to
'2024-02-01' and api_key defaults to '' if missing.

The two duplicate _resolve_azure_credentials definitions in
cv_model.py and embedding_model.py are removed in favor of the
shared helper.

Fixes infiniflow#17675. Supersedes the partial infiniflow#17215 (which only touched the
chat branch and used a silent-fallback helper).
Harsh23Kashyap added a commit to Harsh23Kashyap/ragflow that referenced this pull request Aug 20, 2026
…t a JSON object (infiniflow#17389)

The BaiduYiyan / Qianfan provider in rag/llm/chat_model.py:1189 does an
unguarded `json.loads(key)` followed by `.get("yiyan_ak")` and
`.get("yiyan_sk")`. The provider REQUIRES a JSON key (per
conf/models/baidu.json) but a user pasting a plain Baidu API key like
"bce-v3/ALTAK-.../..." (the most common mistake: copying from the Qianfan
console into a BaiduYiyan field) would crash with
`json.decoder.JSONDecodeError: Expecting value: line 1 column 1 (char 0)`
from inside rag/llm internals, with no indication of what the user did
wrong.

This is the BaiduYiyan equivalent of:

- infiniflow#17204 / PR infiniflow#17215 (Azure-OpenAI)
- infiniflow#17373 / PR infiniflow#17377 (AWS Bedrock)

Add a parallel helper to rag.llm.key_utils that mirrors the Bedrock fix
shape:

```
def _resolve_qianfan_credentials(key):
    # Accepts dict or JSON-string-encoded dict. Returns the dict.
    # On non-JSON input (e.g. plain "bce-v3/..."):
    #   raises ModelException with the required schema in the message
    #   (yiyan_ak + yiyan_sk, conf/models/baidu.json reference).
    # On JSON top-level non-dict (list, string, number):
    #   raises ModelException rather than letting the caller hit
    #   AttributeError on .get("yiyan_ak").
```

Wire BaiduYiyanChat.__init__ (chat_model.py:1189) through the helper
instead of the bare `json.loads(key)`. The downstream
`key.get("yiyan_ak", "")` / `.get("yiyan_sk", "")` calls are
unchanged.

No public API change. No data-model change. No migration.

Fixes infiniflow#17389.
Harsh23Kashyap added a commit to Harsh23Kashyap/ragflow that referenced this pull request Aug 20, 2026
The Bedrock provider requires a JSON key (auth_mode + bedrock_region plus
mode-specific fields per conf/models/bedrock.json), but every call site
in rag/llm/ was doing an unguarded json.loads(key) followed by
.get("auth_mode"). A user pasting a plain AWS access key (the most
common mistake: copying from the AWS console into a Bedrock field) would
crash with json.decoder.JSONDecodeError: Expecting value: line 1 column
1 (char 0) from inside rag/llm internals -- no indication of what the
user did wrong.

This is the Bedrock equivalent of infiniflow#17204 (Azure-OpenAI), which was fixed
in PR infiniflow#17215. Add a parallel helper to rag.llm.key_utils that:

- Accepts a pre-parsed dict (returns verbatim) or a JSON-string-encoded
  dict (parses and returns the dict).
- On non-JSON input, raises a clear ModelException (retryable=False)
  that names the required fields and points at
  conf/models/bedrock.json for the full schema. Operators can
  self-diagnose the mistake without opening a GitHub issue.
- On a JSON top-level type that is not a dict (list, string, number),
  raises the same ModelException family rather than letting the model
  class call .get("auth_mode") on a non-dict and hit
  AttributeError downstream.

This commit adds the helper only. The wire-up into chat_model.py,
cv_model.py, embedding_model.py and rerank_model.py lands in the next
commit; the model classes' existing key.get("auth_mode") /
bedrock_key.get("auth_mode") calls are unchanged.

Fixes infiniflow#17373.
Harsh23Kashyap added a commit to Harsh23Kashyap/ragflow that referenced this pull request Aug 20, 2026
The Bedrock provider requires a JSON key (auth_mode + bedrock_region plus
mode-specific fields per conf/models/bedrock.json), but every call site
in rag/llm/ was doing an unguarded json.loads(key) followed by
.get("auth_mode"). A user pasting a plain AWS access key (the most
common mistake: copying from the AWS console into a Bedrock field) would
crash with json.decoder.JSONDecodeError: Expecting value: line 1 column
1 (char 0) from inside rag/llm internals -- no indication of what the
user did wrong.

This is the Bedrock equivalent of infiniflow#17204 (Azure-OpenAI), which was fixed
in PR infiniflow#17215. Add a parallel helper to rag.llm.key_utils that:

- Accepts a pre-parsed dict (returns verbatim) or a JSON-string-encoded
  dict (parses and returns the dict).
- On non-JSON input, raises a clear ModelException (retryable=False)
  that names the required fields and points at
  conf/models/bedrock.json for the full schema. Operators can
  self-diagnose the mistake without opening a GitHub issue.
- On a JSON top-level type that is not a dict (list, string, number),
  raises the same ModelException family rather than letting the model
  class call .get("auth_mode") on a non-dict and hit
  AttributeError downstream.

This commit adds the helper only. The wire-up into chat_model.py,
cv_model.py, embedding_model.py and rerank_model.py lands in the next
commit; the model classes' existing key.get("auth_mode") /
bedrock_key.get("auth_mode") calls are unchanged.

Fixes infiniflow#17373.
@JinHai-CN

Copy link
Copy Markdown
Contributor

would you please resolve the conflicts?

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

Labels

🐞 bug Something isn't working, pull request that fix bug. 🌈 python Pull requests that update Python code size:M This PR changes 30-99 lines, ignoring generated files. 🧪 test Pull requests that update test cases.

Projects

None yet

Development

Successfully merging this pull request may close these issues.