#240: Fall back to default RAGFlow dataset name when release is missing

[fauad123]

Closes #240

Summary

• Fix RAGFlowClient.upload_document() so the release based dataset_template is only used when metadata.release is present.
• Preserve the caller provided default dataset_name for no release documents instead of resolving to the literal aia-{release}.
• Keep the existing release behavior unchanged, so aia-{release} with release r2 still resolves to aia-r2.
• Add focused regression tests for both the no release fallback and release specific dataset name resolution.
• Added a dedicated test_ragflow_client_dataset_name_resolution.py test file because the existing RAGFlow client tests focus on metadata handling, dataset caching, and upload exception behavior rather than dataset-name resolution.

Testing

• Full rosetta-cli test suite passes with 39 passed.
• src/validate-types.sh passes type validation.

Assumptions

None.

[isolomatov-gd]

Thanks @fauad123.

To fix:

• Also handle dataset_name=None with no release.
• Reject any resolved name still containing {release}.
• Add a test for the dataset_name=None path.
• Note the precedence rule in the docstring.

With the parameter default dataset_name=None and no release, ragflow_client.py:568 still resolves to the literal aia-{release}. Only reachable via a direct client call today, but a resolved name containing {release} is never legitimate, so a guard there closes the class rather than narrowing the condition.

What checks out: the fix is correct for the path that actually breaks, and falling back to dataset_default is the right call rather than failing loudly. Verified by execution — reverting your one-line change makes the new test fail with assert 'aia-{release}' == 'aia', so it genuinely reproduces #240.

The failing triage check is our infra, not your code — already fixed on main by ac9a641e.

[fauad123]

Thanks @fauad123.

To fix:

• Also handle dataset_name=None with no release.
• Reject any resolved name still containing {release}.
• Add a test for the dataset_name=None path.
• Note the precedence rule in the docstring.

With the parameter default dataset_name=None and no release, ragflow_client.py:568 still resolves to the literal aia-{release}. Only reachable via a direct client call today, but a resolved name containing {release} is never legitimate, so a guard there closes the class rather than narrowing the condition.

What checks out: the fix is correct for the path that actually breaks, and falling back to dataset_default is the right call rather than failing loudly. Verified by execution — reverting your one-line change makes the new test fail with assert 'aia-{release}' == 'aia', so it genuinely reproduces #240.

The failing triage check is our infra, not your code — already fixed on main by ac9a641e.

Thanks for the review @isolomatov-gd . I have addressed all four points:

• dataset_name=None with no release now falls back to the configured dataset_default.
• Added a guard that rejects any resolved dataset name still containing {release}.
• Added regression coverage for the dataset_name=None path and the unresolved-placeholder case.
• Documented the dataset name resolution precedence in upload_document().

Verified with the full CLI test suite (43 passed), type validation and manual RAGFlow checks for both the fallback and guard paths.

[isolomatov-gd]

Thanks @fauad123 — all four points are addressed, and the approach is better than what was asked for.

Nothing left to fix.

What checks out, verified by execution against post-#311 main:

• The defect is real and the tests pin it. Reverting to the true pre-fix state — the two-arg _resolve_dataset_name plus the old call-site ternary — fails 3 of your 4 tests with assert 'aia-{release}' == 'aia', which is exactly [ROSETTA] upload_document publishes no-release docs to literal aia-{release} dataset #240.
• Semantics match the authority [ROSETTA] upload_document publishes no-release docs to literal aia-{release} dataset #240 cites. ContentPublisher._resolve_dataset_name returns dataset_default when release is absent; the client now agrees, so both paths resolve identically instead of diverging.
• Moving the logic into _resolve_dataset_name was the right call. With the fix inside the function, restoring the old call-site ternary changes nothing — the bug class is closed at the source rather than at one call site. Independently confirmed: that mutation alone leaves all 4 tests green.
• The guard earns its keep. Dropping if "{release}" in resolved_name fails the placeholder test on its own; dropping the dataset_default fallback fails the dataset_name=None test on its own. Each assertion is load-bearing.
• Wiring dataset_default through cli.py is necessary, not scope creep. RAGFLOW_DATASET_DEFAULT existed in config but never reached the client, so the fallback would have been hardcoded to "aia" without it. The dataset_default: str = "aia" parameter default keeps every other construction site working.
• 56 passed on the full rosetta-cli suite, mypy clean on 22 files.

One behavioural note, not a defect: with a dataset_template that has no {release} and dataset_name=None, resolution now yields dataset_default where it previously yielded dataset_template. Only reachable through a direct client call, it matches the precedence you documented, and dataset_default is the safer answer for what is really a misconfiguration.

I pushed a merge of main into your branch to clear a conflict — a one-line kwarg collision in the RAGFlowClient(...) call, where #311 added timeout=config.timeout and yours adds dataset_default=config.dataset_default. Both kept; that conflict was ours, not yours.

[isolomatov-gd]

Approved. Verified by execution: genuine repro of #240, semantics match ContentPublisher, every new assertion independently load-bearing under mutation. 56 passed, mypy clean.

[fauad123]

@isolomatov-gd Thanks for the detailed review and verification!