You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
{{ message }}
Repository navigation
fix(ingestion): resolve tenant_validation import in prod image - #3793
Supersedes #3774 (same commit c38f065c8ac19f12ee4e390a5437d958fa6b9c2b on an engine-prefix branch so Engine Review Pass accepts the author). Hermes PASS on #3774: #3774 (comment)
Summary
Production CSV and EDI ingest returned 500 because services/ingestion/app/webhook_compat.py used an absolute from app.tenant_validation import. Under the consolidated server / Docker PYTHONPATH layout, top-level app resolves to another service's package (compliance/graph/...), not ingestion.
Fix
Switch to a package-relative from .tenant_validation import validate_tenant_id.
Origin: uncommitted patch from C.Lee's interactive Claude session on Mac 33e1b4b0 (webhook_compat.py +14/-4). Kept only the import-path fix; dropped the unrelated canonical dual-write edits in that working-tree diff.
No other absolute from app.tenant_validation call sites in ingestion app code. Left labels/gs1_128.py's unrelated from app.gs1_identifiers alone (not the CSV/EDI 500 path; not payment/security/receipt).
Test plan
New regression test reproduces server/main.py path bootstrap, asserts absolute import fails, and runs recorded fixtures shipping_pass_partner.csv and edi_corpus/asn_856.edi through ingest_events (and CSV/EDI HTTP routes) without a 500.
Related suite slice (receipt / webhook auth / tenant_validation_1342) — 76 passed
Docker image build not available on the box; test replicates prod PYTHONPATH / path order instead.
Hermes review requested.
Mac working tree left untouched (diff copied out only; no commit/stash/reset on the Mac).
Note
Low Risk
Single import-path change on the ingest hot path; behavior is unchanged when the correct module loads, and regression tests cover the prod layout.
Overview
Fixes production 500s on CSV and EDI ingest caused by ingest_events importing tenant validation via top-level from app.tenant_validation, which resolves to another service’s app package in the consolidated server/Docker PYTHONPATH layout.
webhook_compat.py now uses from .tenant_validation import validate_tenant_id so validation always loads ingestion’s module, with a short comment explaining the collision.
Adds test_prod_import_path_tenant_validation.py, which mirrors server/main.py path ordering, confirms the old absolute import fails, and runs recorded CSV/EDI fixtures through ingest_events and the CSV/EDI HTTP routes without a 500.
Reviewed by Cursor Bugbot for commit b2a8773. Bugbot is set up for automated code reviews on this repo. Configure here.
Absolute ``from app.tenant_validation`` inside webhook_compat.ingest_events
resolved to another service's ``app`` package under the consolidated
server/PYTHONPATH layout, so CSV and EDI ingest returned 500 in production.
Use a package-relative import instead. Origin: uncommitted patch from
C.Lee's interactive Claude session on Mac 33e1b4b0 (webhook_compat.py
+14/-4); kept only the import-path fix and dropped the unrelated
canonical dual-write edits. Adds a regression test that reproduces the
prod path bootstrap and exercises the recorded shipping CSV and 856 EDI
fixtures.
No actionable comments were generated in the recent review.
Walkthrough
The ingestion helper now imports tenant validation through its package-relative module path. Regression tests simulate the production package collision and exercise CSV and EDI ingestion and upload routes using recorded fixtures.
Tenant validation now resolves within ingestion, and regression tests cover the collision. No merge-blocking issue was established.
Pre-merge checks: 3 · 1 · 1
Failed checks: 1 warning · 1 inconclusive
Check name
Status
Explanation
Resolution
Docstring Coverage
Docstring coverage is 57.14% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff with cyclomatic complexity of at least 2. Documenta…
Write docstrings for the functions missing them to satisfy the coverage threshold.
Description check
The description covers the template's required content. Summary: it explains the production 500 cause and the fix, with bullet-style detail. Test Plan: it lists the new regression test, the pytest com…
1) Tick the Type of Change box that applies (Bug fix). 2) Tick the Test Plan boxes that apply (unit tests added, and tests run). 3) Address the checklist. Because the new test uses sys.modules stubs and monkeypatching, justify the sys.modul…
Passed checks: 3 passed
Check name
Status
Explanation
Title check
The title "fix(ingestion): resolve tenant_validation import in prod image" matches the main change. The diff switches webhook_compat.py from the absolute app.tenant_validation import to a package-rela…
Linked Issues check
Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check
Check skipped because no linked issues were found for this pull request.
Full details: Title check
Explanation
The title "fix(ingestion): resolve tenant_validation import in prod image" matches the main change. The diff switches webhook_compat.py from the absolute app.tenant_validation import to a package-relative import, which fixes the production CSV and EDI ingest 500s. The title is short, uses a conventional commit prefix, and names the component and the problem area. It does not name the specific mechanism (relative import), but that is an acceptable level of detail.
Full details: Docstring Coverage
Explanation
Docstring coverage is 57.14% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff with cyclomatic complexity of at least 2. Documentation-only edits and deletions within surviving functions are included; unchanged diff context and fully deleted functions are excluded. Analyzed 7 functions across 2 files.
Resolution
Write docstrings for the functions missing them to satisfy the coverage threshold.
Full details: Description check
Explanation
The description covers the template's required content. Summary: it explains the production 500 cause and the fix, with bullet-style detail. Test Plan: it lists the new regression test, the pytest command with 3 passed, a related suite run with 76 passed, and the reason Docker build was not run. Related Issues is not filled in. It does not mark the Type of Change or the Test Plan checkboxes. The Checklist is not addressed. The checklist asks the author to justify any sys.modules[X] = stub use in the description. The new test stubs database persistence, authentication, rate limiting, deduplication, and rules dependencies, and the description does not give that justification. The DeepSeek comment also flags that the test does not restore sys.path, sys.modules, or the patched validate_tenant_id.
Resolution
Tick the Type of Change box that applies (Bug fix). 2) Tick the Test Plan boxes that apply (unit tests added, and tests run). 3) Address the checklist. Because the new test uses sys.modules stubs and monkeypatching, justify the sys.modules stubs in the PR description, as the template requires. 4) Fill in the Related Issues section, or state that there is none. 5) Remove the Cursor/Bugbot and Hermes-session boilerplate and the 'Supersedes #3774' note if they are not needed, or keep them as context. 6) Confirm the checklist items for code patterns, secrets, docs, and lint warnings.
Finishing Touches
Generate docstrings
Commit to this branch
Create a new PR
Generate unit tests (beta)
Commit to this branch
Create a new PR
Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts
CodeRabbit Verify
Run CodeRabbit Verify to test the changes in this pull request.
Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 4 remain after this review.
Same head SHA I passed on #3774 (#3774 (comment)), re-opened unchanged on claude/ingest-import-path. Same two files: services/ingestion/app/webhook_compat.py and services/ingestion/tests/test_prod_import_path_tenant_validation.py. Prior findings stand: the relative import is correct; 3 new tests and 99 related tests passed. Non-blocking caveat from before: the test's sys.path order doesn't match the Dockerfile's PYTHONPATH order.
detect-secrets Secret Keyword flagged x_regengine_api_key="test-key"
in the new prod-path regression test (PR Gate: Secrets). Mark the
fixture strings with pragma allowlist; not a real credential.
Signed-off-by: C.Lee Sellers <131207452+PetrefiedThunder@users.noreply.github.com>
Signed-off-by: Christopher <131207452+PetrefiedThunder@users.noreply.github.com>
Bring in post-#3773 main (KDE/FTL #3772, evidence policy #3717, frontend audit #3770, prod migration approval #3765) so PR #3793 CI is not stale again.
Signed-off-by: C.Lee Sellers <lee@regengine.com>
Engine: DeepSeek (deepseek-v4-pro via API), run Nov 3, 2026 ~12:45 AM PT, on the full PR diff (merge-base b1e5f815..6d6c8ea9). The verdict is DeepSeek's.
Author check:ed48d785, which adds # pragma: allowlist secret to the 3 synthetic x_regengine_api_key="test-key" fixture lines, was written by Christopher (PetrefiedThunder) and signed off by C.Lee. DeepSeek did not write it. The other commits since Hermes PASS c38f065c are main merges (83c90f08, 6d6c8ea9). The PR still changes only services/ingestion/app/webhook_compat.py and services/ingestion/tests/test_prod_import_path_tenant_validation.py.
Local: the 3 tests in test_prod_import_path_tenant_validation.py pass on this head. CI is green except Engine Review Pass, which is red because no engine PASS existed at this head yet.
DeepSeek findings (all non-blocking, test hygiene):
test_prod_import_path_tenant_validation.py:72: _bootstrap_prod_sys_path() changes the global sys.path and never restores it, so later tests can depend on run order. Use monkeypatch.setattr(sys, "path", ...) or snapshot and restore it.
:65: removing app / app.* from sys.modules is never undone. Save the removed entries and restore them at fixture teardown.
:291: tv_mod.validate_tenant_id = _tracking_validate replaces a module global and never puts it back. Use monkeypatch.setattr(tv_mod, "validate_tenant_id", _tracking_validate).
Require both CSV and EDI HTTP routes to reach ingest_events (assert
csv_upload and edi_856_inbound in seen); POST EDI at /api/v1/ingest/edi.
Restore sys.path via monkeypatch and reinstall purged app modules at
fixture teardown; patch validate_tenant_id with monkeypatch.setattr.
Signed-off-by: C.Lee Sellers <131207452+PetrefiedThunder@users.noreply.github.com>
Signed-off-by: Christopher <131207452+PetrefiedThunder@users.noreply.github.com>
…h harden
Bring in 6d6c8ea (merge main into claude/ingest-import-path) under the
local test hygiene commit so the tip can plain-push.
Signed-off-by: C.Lee Sellers <131207452+PetrefiedThunder@users.noreply.github.com>
Signed-off-by: Christopher <131207452+PetrefiedThunder@users.noreply.github.com>
Engine: DeepSeek, delta re-review from prior PASS 6d6c8ea9 (issuecomment-6599006457) → exact head b2a87737ccaf842c0dd6ad8ae04e2ed825b78d95, Nov 3, 2026 ~1:24 AM PT. Worktree /workspace/RegEngine-ingest-import-path.
b2a87737 merge of that harden onto prior 6d6c8ea9 tip
Merge identity: HEAD is a 2-parent merge (1e53ede + 6d6c8ea9). git merge-tree --write-tree equals HEAD^{tree} (4422c265…). Only file touched vs prior PASS: services/ingestion/tests/test_prod_import_path_tenant_validation.py (+29/−17).
Prior nits / CodeRabbit majors — verified fixed:
sys.path now set via monkeypatch.setattr(sys, "path", …) in _bootstrap_prod_sys_path.
Purged app / app.* modules saved and restored in fixture finally.
validate_tenant_id patched with monkeypatch.setattr(tv_mod, …) (no bare global assign).
Route coverage tightened: both CSV and EDI HTTP paths must reach ingest_events (csv_upload / edi_856_inbound in seen); EDI posts to /api/v1/ingest/edi.
Synthetic key / allowlist (unchanged, still scoped to fixture):
x_regengine_api_key="test-key" with # pragma: allowlist secret — synthetic test fixture only on the fixture call sites only.
Author of allowlist commit ed48d785 remains PetrefiedThunder (not deepseek).
CI on this exact head: green except ignored gate-only checks (Engine Review Pass / Head SHA / PR Blocker Checklist). Notably: Ingestion full suite, Test ingestion, Repo-root security/RLS/isolation, Live DB, Tenant Isolation & Auth, PR Gate: Backend, Backend CI Status Gate, PR Gate: Secrets — SUCCESS.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
PetrefiedThunder
left a comment
Supersedes #3774 (same commit
c38f065c8ac19f12ee4e390a5437d958fa6b9c2bon an engine-prefix branch so Engine Review Pass accepts the author). Hermes PASS on #3774: #3774 (comment)Summary
Production CSV and EDI ingest returned 500 because
services/ingestion/app/webhook_compat.pyused an absolutefrom app.tenant_validationimport. Under the consolidated server / DockerPYTHONPATHlayout, top-levelappresolves to another service's package (compliance/graph/...), not ingestion.Fix
from .tenant_validation import validate_tenant_id.33e1b4b0(webhook_compat.py+14/-4). Kept only the import-path fix; dropped the unrelated canonical dual-write edits in that working-tree diff.from app.tenant_validationcall sites in ingestion app code. Leftlabels/gs1_128.py's unrelatedfrom app.gs1_identifiersalone (not the CSV/EDI 500 path; not payment/security/receipt).Test plan
server/main.pypath bootstrap, asserts absolute import fails, and runs recorded fixturesshipping_pass_partner.csvandedi_corpus/asn_856.edithroughingest_events(and CSV/EDI HTTP routes) without a 500.pytest services/ingestion/tests/test_prod_import_path_tenant_validation.py— 3 passedPYTHONPATH/ path order instead.Hermes review requested.
Mac working tree left untouched (diff copied out only; no commit/stash/reset on the Mac).
Note
Low Risk
Single import-path change on the ingest hot path; behavior is unchanged when the correct module loads, and regression tests cover the prod layout.
Overview
Fixes production 500s on CSV and EDI ingest caused by
ingest_eventsimporting tenant validation via top-levelfrom app.tenant_validation, which resolves to another service’sapppackage in the consolidated server/DockerPYTHONPATHlayout.webhook_compat.pynow usesfrom .tenant_validation import validate_tenant_idso validation always loads ingestion’s module, with a short comment explaining the collision.Adds
test_prod_import_path_tenant_validation.py, which mirrorsserver/main.pypath ordering, confirms the old absolute import fails, and runs recorded CSV/EDI fixtures throughingest_eventsand the CSV/EDI HTTP routes without a 500.Reviewed by Cursor Bugbot for commit b2a8773. Bugbot is set up for automated code reviews on this repo. Configure here.