feat: Match connector_file_id in ACL/metadata updates#1704
Merged
lucaseduoli merged 1 commit intoMay 28, 2026
Conversation
Ensure ACL and metadata updates target both Langflow and non-Langflow chunks by matching document IDs against multiple fields. Introduce _build_id_query and id_fields parameters to should_update_acl, update_document_acl, and batch_update_acls so updates can match document_id and connector_file_id. Stop overwriting chunk owner during ACL syncs (only allowed_users/allowed_groups are updated) and set owner at ingest to the syncing user in TaskProcessor. Adjust tests to expect the combined bool query and make minor typing/import/formatting cleanups.
Contributor
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
github-actions
Bot
added
backend
lucaseduoli
approved these changes
May 28, 2026
lucaseduoli
merged commit May 28, 2026
a43f140
into
feat/langflow_ingestion_disable
10 of 12 checks passed
Contributor
There was a problem hiding this comment.
Pull request overview
This PR improves connector-driven ACL and metadata synchronization so updates apply to both Langflow and non-Langflow chunk variants by matching the stable connector document ID across multiple fields (e.g., document_id and connector_file_id). It also adjusts ownership semantics so ACL syncs don’t reassign chunk ownership, and sets chunk owner at ingest time to the syncing user.
Changes:
- Add multi-field ID matching (
_build_id_query,id_fields) for ACL update/check paths so connector updates can target bothdocument_idandconnector_file_id. - Stop updating
ownerduring ACL sync; setownerat ingest to the syncing/uploading user and only syncallowed_users/allowed_groups. - Update tests and connector metadata update query to expect/use a
bool/shouldquery across both ID fields.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 7 comments.
| File | Description |
|---|---|
| tests/unit/connectors/test_update_connector_metadata_filename.py | Updates assertions to expect a combined bool query matching both document_id and connector_file_id. |
| src/utils/acl_utils.py | Introduces _build_id_query and id_fields plumbing; removes owner writes during ACL updates; refactors typing/formatting. |
| src/models/processors.py | Sets chunk owner to the syncing user at ingest; limits ACL ingestion to access lists only. |
| src/connectors/service.py | Passes id_fields into ACL updates and expands metadata update query to match both ID fields. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| import asyncio | ||
| from typing import Dict, List, Tuple, Optional | ||
|
|
||
| from src.connectors.base import DocumentACL |
Comment on lines
28
to
+33
| acl_data = { | ||
| "owner": acl.owner, | ||
| "allowed_users": sorted(acl.allowed_users), | ||
| "allowed_groups": sorted(acl.allowed_groups), | ||
| } | ||
| return hashlib.sha256( | ||
| json.dumps(acl_data, sort_keys=True).encode() | ||
| ).hexdigest() | ||
| return hashlib.sha256(json.dumps(acl_data, sort_keys=True).encode()).hexdigest() |
Comment on lines
74
to
80
| try: | ||
| # Query one chunk for this document | ||
| response = await opensearch_client.search( | ||
| index="documents", | ||
| body={ | ||
| "query": {"term": {"document_id": document_id}}, | ||
| "query": _build_id_query(document_id, id_fields), | ||
| "size": 1, |
Comment on lines
113
to
119
| async def update_document_acl( | ||
| document_id: str, | ||
| acl: DocumentACL, | ||
| opensearch_client | ||
| ) -> Dict[str, any]: | ||
| opensearch_client, | ||
| id_fields: tuple[str, ...] = ("document_id",), | ||
| ) -> dict[str, any]: | ||
| """ |
Comment on lines
174
to
179
| async def batch_update_acls( | ||
| acl_updates: List[Tuple[str, DocumentACL]], | ||
| opensearch_client | ||
| ) -> Dict[str, any]: | ||
| acl_updates: list[tuple[str, DocumentACL]], | ||
| opensearch_client, | ||
| id_fields: tuple[str, ...] = ("document_id",), | ||
| ) -> dict[str, any]: | ||
| """ |
Comment on lines
150
to
151
| response = await opensearch_client.update_by_query( | ||
| index="documents", |
Comment on lines
224
to
+229
| # Bulk update chunks for each document (parallelized) | ||
| update_tasks = [ | ||
| opensearch_client.update_by_query( | ||
| index="documents", | ||
| body={ | ||
| "query": {"term": {"document_id": doc_id}}, | ||
| "query": _build_id_query(doc_id, id_fields), |
lucaseduoli
added a commit
that referenced
this pull request
May 28, 2026
* Added disable langflow ingestion setting on backend * Added option to disable langflow ingestion in frontend * Fixed ingestion not going to tasks * style: ruff format (auto) * fix frontend lint errors * fixed type errors * Potential fix for pull request finding 'CodeQL / Information exposure through an exception' Co-authored-by: Copilot Autofix powered by AI <62310815+github-advanced-security[bot]@users.noreply.github.com> * fixed mypy findings * fixed lint * fix: apply CodeRabbit auto-fixes Fixed 4 file(s) based on 6 unresolved review comments. Co-authored-by: CodeRabbit <noreply@coderabbit.ai> * fix: apply CodeRabbit auto-fixes Fixed 2 file(s) based on 1 unresolved review comment. Co-authored-by: CodeRabbit <noreply@coderabbit.ai> * style: ruff format (auto) * fix mypi * added acl support * style: ruff format (auto) * fix mypy * fixed integration tests * pass temp file paths as empty to not delete default files on error * wait for opensearch * fix session manager jwt * fix lint * fix spaces * fix health check * fix kwargs * style: ruff format (auto) * fixed nitpicks from coderabbit * style: ruff format (auto) * fix ruff * fix result * fix types * Match connector_file_id in ACL/metadata updates (#1704) Ensure ACL and metadata updates target both Langflow and non-Langflow chunks by matching document IDs against multiple fields. Introduce _build_id_query and id_fields parameters to should_update_acl, update_document_acl, and batch_update_acls so updates can match document_id and connector_file_id. Stop overwriting chunk owner during ACL syncs (only allowed_users/allowed_groups are updated) and set owner at ingest to the syncing user in TaskProcessor. Adjust tests to expect the combined bool query and make minor typing/import/formatting cleanups. * mypy fix --------- Co-authored-by: autofix-ci[bot] <114827586+autofix-ci[bot]@users.noreply.github.com> Co-authored-by: Copilot Autofix powered by AI <62310815+github-advanced-security[bot]@users.noreply.github.com> Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com> Co-authored-by: CodeRabbit <noreply@coderabbit.ai> Co-authored-by: Edwin Jose <edwin.jose@datastax.com> Co-authored-by: Edwin Jose <edwinjose900@gmail.com>
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Ensure ACL and metadata updates target both Langflow and non-Langflow chunks by matching document IDs against multiple fields. Introduce _build_id_query and id_fields parameters to should_update_acl, update_document_acl, and batch_update_acls so updates can match document_id and connector_file_id. Stop overwriting chunk owner during ACL syncs (only allowed_users/allowed_groups are updated) and set owner at ingest to the syncing user in TaskProcessor. Adjust tests to expect the combined bool query and make minor typing/import/formatting cleanups.