Skip to content

fix: remove leftover refactor scaffolding - #892

Merged
Ahmath-Gadji merged 1 commit into
developfrom
fix/888-remove-leftover-refactor-scaffolding
Sep 3, 2026
Merged

fix: remove leftover refactor scaffolding#892
Ahmath-Gadji merged 1 commit into
developfrom
fix/888-remove-leftover-refactor-scaffolding

Conversation

@Ahmath-Gadji

@Ahmath-Gadji Ahmath-Gadji commented Sep 2, 2026

Copy link
Copy Markdown
Collaborator

Summary

  • Deletes openrag/consts.py (a re-export shim outside the layer roots, which hid its four importers from check_layer_imports.py), the unused SingletonMeta metaclass, and the empty services/events/ package
  • Repoints the four shim consumers (api/dependencies/llm.py, api/dependencies/files.py, api/routers/user/chat.py, app_front.py) directly at core.utils.consts
  • Renames core/utils/conts.py -> consts.py to fix the module name typo while its importers are already being touched

Closes #888

Test plan

  • uv run ruff check openrag/ tests/ / ruff format --check
  • python scripts/check_layer_imports.py passes
  • uv run pytest tests/unit/ — 2764 passed (1 pre-existing, unrelated failure in test_compose_storage.py from a local docker-compose bind-mount edit, not part of this branch)
  • Verified at runtime that import consts now raises ModuleNotFoundError and all repointed modules import cleanly

Summary by CodeRabbit

  • New Features

    • Added centralized handling for shared constants and metadata sanitization.
    • Added protection against exposing or spoofing server-managed metadata fields.
  • Bug Fixes

    • Corrected module references for constants and metadata utilities, improving reliability across uploads, indexing, chat, and search workflows.
  • Refactor

    • Consolidated shared constants under a common utility module.
    • Removed legacy constant re-exports and singleton enforcement.

@coderabbitai

coderabbitai Bot commented Sep 2, 2026

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Team

Run ID: 62cc4706-0712-44e8-8e24-c4063273dd31

📥 Commits

Reviewing files that changed from the base of the PR and between 5687cd7 and e75749b.

📒 Files selected for processing (16)
  • openrag/api/dependencies/files.py
  • openrag/api/dependencies/llm.py
  • openrag/api/routers/user/chat.py
  • openrag/app_front.py
  • openrag/consts.py
  • openrag/core/prompts/vlm_prompt_builder.py
  • openrag/core/utils/consts.py
  • openrag/core/utils/singleton.py
  • openrag/services/events/__init__.py
  • openrag/services/orchestrators/conversion_service.py
  • openrag/services/orchestrators/indexing_service.py
  • openrag/services/orchestrators/mcp_service.py
  • openrag/services/orchestrators/partition_service.py
  • openrag/services/storage/vector_store_searcher.py
  • openrag/services/workers/dispatcher.py
  • tests/unit/core/prompts/test_vlm_prompt_builder.py
💤 Files with no reviewable changes (2)
  • openrag/core/utils/singleton.py
  • openrag/consts.py

Included review availability: Your plan provides up to 2 included reviews per hour; 1 remains after this review.


📝 Walkthrough

Walkthrough

The PR centralizes constants and metadata helpers in core.utils.consts, updates importers, removes legacy constant re-exports and unused singleton scaffolding, and corrects misspelled module references.

Changes

Constants module migration

Layer / File(s) Summary
Shared constants and metadata helpers
openrag/core/utils/consts.py, openrag/consts.py, openrag/core/utils/singleton.py
The new module defines shared constants and metadata-sanitizing helpers. Legacy constant re-exports and the unused SingletonMeta implementation are removed.
Importer migration and validation
openrag/api/..., openrag/app_front.py, openrag/core/prompts/..., openrag/services/..., tests/unit/core/prompts/...
Application code and tests now import from core.utils.consts. Misspelled core.utils.conts references are corrected.

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

Merge Risk: ⚪ Minimal · up to e7574

The PR only cleans up unused scaffolding and redirects imports to the correctly named internal constants module without changing runtime behavior; no actionable merge-blocking risk remains after normal checks and review.

Suggested reviewers: hedhoud, enjoybacon7

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 50.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 6 functions across 13 files. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly identifies the primary change: removing leftover refactor scaffolding.
Linked Issues check ✅ Passed The changes satisfy issue #888 by removing the re-export shim and unused SingletonMeta, updating consumers, and correcting conts.py to consts.py. The provided objectives also state that the empty serv…
Out of Scope Changes check ✅ Passed The reported changes remain within issue #888. Import corrections, constant-module cleanup, and removal of unused scaffolding support the stated refactor cleanup.
Full details: Linked Issues check

Explanation

The changes satisfy issue #888 by removing the re-export shim and unused SingletonMeta, updating consumers, and correcting conts.py to consts.py. The provided objectives also state that the empty services/events/ package was deleted.

  • Fix all pre-merge checks with AI
✨ Finishing Touches 💡 1
📝 Generate docstrings 💡
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/888-remove-leftover-refactor-scaffolding

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.

Deletes three dead artifacts from the hexagonal migration: the
openrag/consts.py re-export shim (which also hid its four importers
from check_layer_imports.py, since the shim sits outside the layer
roots), the unused SingletonMeta metaclass, and the empty
services/events/ package.

Repoints the four shim consumers directly at core.utils.consts, and
renames core/utils/conts.py -> consts.py to fix the module name typo
while its importers are already being touched.
@Ahmath-Gadji
Ahmath-Gadji force-pushed the fix/888-remove-leftover-refactor-scaffolding branch from e75749b to 2dc287e Compare September 3, 2026 07:43

@EnjoyBacon7 EnjoyBacon7 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Reviewed the full diff. Mechanically complete and behavior-preserving.

Verified:

  • No surviving references to conts, SingletonMeta, utils.singleton, or services.events anywhere in the repo, including docs, .github/, infra/ and conf/.
  • AST-based import resolution across openrag/ and tests/: zero unresolvable internal imports.
  • No dynamic references that a symbol grep would miss (importlib, sys.modules stubbing, monkeypatch/patch string targets, spec_from_file_location). tests/unit/test_app_front_secret.py loads app_front.py by path but only syspath_prepends openrag/, so the new import resolves identically.
  • All attributes used via the widened module import exist (FILE_READ_CHUNK_SIZE, PARTITION_PREFIX, LEGACY_PARTITION_PREFIX). The shim only re-exported 4 names, so this is strictly additive, with no shadowing and no cycle risk.
  • scripts/check_layer_imports.py passes. The repoint turns an invisible top-level import into a properly classified api -> core edge.
  • ruff check and ruff format --check both clean.
  • Packaging unaffected: pyproject uses packages.find include = ["openrag*"], so removing services/events needs no manifest edit.

Fully closes #888.

Minor note, not blocking and pre-existing: consts.py also holds behavior (is_internal_metadata_key, strip_internal_metadata, strip_protected_metadata), which is an odd fit for a constants module.

@Ahmath-Gadji
Ahmath-Gadji merged commit c53261a into develop Sep 3, 2026
6 checks passed
@Ahmath-Gadji
Ahmath-Gadji deleted the fix/888-remove-leftover-refactor-scaffolding branch September 3, 2026 10:23
@Ahmath-Gadji Ahmath-Gadji added the fix Fix issue label Sep 3, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

fix Fix issue

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Leftover refactor scaffolding: openrag/consts.py shim with 4 consumers, unused SingletonMeta, empty services/events/

2 participants