Add picture description support for document processing - #323
Open
gshikhar2021 wants to merge 1 commit into
Open
Add picture description support for document processing#323gshikhar2021 wants to merge 1 commit into
gshikhar2021 wants to merge 1 commit into
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Enterprise Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughSummary by CodeRabbit
WalkthroughThe change adds picture-description API configuration to public types and CRD schemas. The controller reads ChangesPicture description API
Estimated code review effort: 3 (Moderate) | ~20 minutes Sequence Diagram(s)sequenceDiagram
participant OperatorSecret
participant ControllerReconciliation
participant DoclingClient
participant PictureDescriptionAPI
OperatorSecret->>ControllerReconciliation: Provide VLM_API_KEY
ControllerReconciliation->>ControllerReconciliation: Convert picture-description settings
ControllerReconciliation->>DoclingClient: Pass URL, params, prompt, timeout, concurrency, headers
DoclingClient->>PictureDescriptionAPI: Use configured API endpoint
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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 `@internal/controller/controllerconfig_controller.go`:
- Around line 143-145: Replace the process-global VLM API key assignment in
internal/controller/controllerconfig_controller.go lines 143-145 with a
synchronized, namespace-scoped configuration snapshot keyed by req.Namespace.
Update internal/controller/documentprocessor_controller.go lines 126-130 to
retrieve only the credential associated with the reconciled resource namespace,
preserving synchronization for concurrent reconciliation access.
🪄 Autofix
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: Enterprise
Run ID: 08e4b646-763a-4470-82cb-ed464990342c
📒 Files selected for processing (10)
api/v1alpha1/documentprocessor_types.goapi/v1alpha1/zz_generated.deepcopy.goconfig/crd/bases/operator.dataverse.redhat.com_documentprocessors.yamlconfig/crd/bases/operator.dataverse.redhat.com_unstructureddatapipelines.yamlconfig/samples/operator_v1alpha1_unstructureddatapipeline_gdrive.yamlconfig/samples/unstructured-secret.yamlinternal/controller/controllerconfig_controller.gointernal/controller/documentprocessor_controller.gopkg/docling/client.gopkg/docling/zz_generated.deepcopy.go
💤 Files with no reviewable changes (1)
- pkg/docling/zz_generated.deepcopy.go
Comment on lines
+143
to
+145
| // VLM API key for picture description | ||
| vlmAPIKey = string(secret.Data["VLM_API_KEY"]) | ||
|
|
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | 🏗️ Heavy lift
Keep VLM_API_KEY namespace-scoped and synchronized.
VLM_API_KEY comes from a Secret in req.Namespace, but Line 144 stores it in a process-global variable. A reconciliation in namespace B can overwrite the key before a DocumentProcessor reconciliation in namespace A reads it. The processor can then use namespace B's credential. Concurrent reads and writes also cause a Go data race.
Replace the global with a namespace-scoped, synchronized configuration snapshot, or resolve the applicable Secret during DocumentProcessor reconciliation.
internal/controller/controllerconfig_controller.go#L143-L145: store the key with its namespace instead of assigning a process-global value.internal/controller/documentprocessor_controller.go#L126-L130: retrieve only the key for the reconciled resource namespace.
As per path instructions, focus on major issues impacting performance, readability, maintainability and security.
📍 Affects 2 files
internal/controller/controllerconfig_controller.go#L143-L145(this comment)internal/controller/documentprocessor_controller.go#L126-L130
🤖 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 `@internal/controller/controllerconfig_controller.go` around lines 143 - 145,
Replace the process-global VLM API key assignment in
internal/controller/controllerconfig_controller.go lines 143-145 with a
synchronized, namespace-scoped configuration snapshot keyed by req.Namespace.
Update internal/controller/documentprocessor_controller.go lines 126-130 to
retrieve only the credential associated with the reconciled resource namespace,
preserving synchronization for concurrent reconciliation access.
Source: Path instructions
Enable Gemini as a vision language model in the document processor to generate descriptions of images found in PDFs. The API key is read from the operator secret and injected at runtime
gshikhar2021
force-pushed
the
image-description
branch
from
August 11, 2026 11:39
83f3163 to
45b6ec1
Compare
| logger := log.FromContext(ctx) | ||
| client := &http.Client{ | ||
| Timeout: 15 * time.Second, | ||
| Timeout: 60 * time.Second, |
Contributor
There was a problem hiding this comment.
Any reason why are we changing this ?
Member
There was a problem hiding this comment.
also if this is needed then we should take this value from controllerconfig so that same can be fine tuned
vinamra28
reviewed
Aug 19, 2026
| Prompt string `json:"prompt,omitempty"` | ||
| Timeout string `json:"timeout,omitempty"` | ||
| Concurrency int `json:"concurrency,omitempty"` | ||
| Headers map[string]string `json:"headers,omitempty"` |
Member
There was a problem hiding this comment.
what kind of headers needs to be passed here?
vinamra28
reviewed
Aug 19, 2026
| } | ||
|
|
||
| type PictureDescriptionAPI struct { | ||
| URL string `json:"url"` |
Member
There was a problem hiding this comment.
is this going to be constant for each pipeline? If so, can we move this to controllerconfig?
concaf
reviewed
Aug 19, 2026
Comment on lines
+42
to
+48
| Extract all visible content from this image: text | ||
| (exact wording, spelling, numbers, dates, percentages), | ||
| table data (preserve rows, columns, headers, values), | ||
| chart/graph data (axis labels, data points, legends, trends), | ||
| diagrams (labels, connections, flow direction), | ||
| and any logos or watermarks. | ||
| Reproduce numbers and figures exactly as shown. |
Contributor
There was a problem hiding this comment.
more detailed prompt needed
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
4 participants
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.
Enable Gemini as a vision language model in the document processor to generate descriptions of images found in PDFs. The API key is read from the operator secret and injected at runtime