Skip to content

Setting#66

Merged
namtroi merged 2 commits into
mainfrom
setting
Dec 28, 2025
Merged

Setting#66
namtroi merged 2 commits into
mainfrom
setting

Conversation

@namtroi

@namtroi namtroi commented Dec 28, 2025

Copy link
Copy Markdown
Owner

PR Type

Enhancement, Bug fix


Description

  • Unified profile create/duplicate dialogs with shared field configuration

  • Fixed profile delete bug: read confirmation from query param and body

  • Added duplicate name validation with inline error display

  • Disabled Hugging Face progress bars to prevent tqdm thread safety bug

  • Removed AI worker concurrency settings, simplified to BullMQ control

  • Improved dialog UI: reduced spacing, standardized fonts, added tooltips


Diagram Walkthrough

flowchart LR
  A["Profile Management"] --> B["ProfileFormDialog<br/>Create/Duplicate"]
  A --> C["ProfileCard<br/>View Settings"]
  B --> D["profile-field-config.ts<br/>Shared Labels/Tooltips"]
  C --> D
  E["Backend Delete"] --> F["Read confirmed<br/>from query + body"]
  G["AI Worker"] --> H["Disable HF<br/>Progress Bars"]
  I["Config Cleanup"] --> J["Remove MAX_WORKERS<br/>Use PDF_CONCURRENCY"]
Loading

File Walkthrough

Relevant files
Bug fix
2 files
profile-routes.ts
Fix profile delete confirmation handling                                 
+6/-1     
main.py
Disable Hugging Face progress bars                                             
+5/-0     
Error handling
1 files
client.ts
Preserve error code in API responses                                         
+4/-2     
Enhancement
5 files
profile-field-config.ts
Create shared profile field configuration                               
+125/-0 
ProfileFormDialog.tsx
Unified create/duplicate dialog with validation                   
+387/-0 
ProfileCard.tsx
Use shared field config and callback props                             
+51/-51 
ProfilePage.tsx
Update dialog state management for unified form                   
+27/-8   
pdf_converter.py
Remove semaphore-based concurrency limiting                           
+1/-11   
Miscellaneous
1 files
ProfileCreateDialog.tsx
Remove old create dialog file                                                       
+0/-266 
Configuration changes
2 files
config.py
Remove max_workers setting and validator                                 
+1/-14   
.env.example
Simplify environment variables configuration                         
+8/-21   
Documentation
2 files
extension-processing-profile.md
Document profile schema and settings changes                         
+47/-2   
processing-settings.md
Update concurrency documentation                                                 
+2/-11   

Frontend:
- Create unified ProfileFormDialog for both create and duplicate modes
- Fix OCR mode options: 'off' -> 'never' to match backend schema
- Update ProfilePage with dialog state management
- Update ProfileCard to use callback prop for duplicate
- Remove old ProfileCreateDialog
- Optimize dialog UI: reduce spacing, remove Embedding section
- Standardize font sizes in ProfileCard settings display

Backend (AI Worker):
- Disable HF progress bars to prevent tqdm._lock thread safety bug
  when multiple workers download models concurrently
…abels/tooltips

- Fix profile delete bug: read 'confirmed' from both query param and body
- Add duplicate name error display in ProfileFormDialog with inline validation
- Preserve error code in API client for better error handling
- Create shared profile-field-config.ts for labels and tooltips
- Update ProfileCard to use shared field config
- Add tooltips to ProfileFormDialog with position-aware placement
- Fix tooltip overflow: left items show right, right items show left
@namtroi namtroi merged commit 8c33572 into main Dec 28, 2025
7 checks passed
@qodo-code-review

Copy link
Copy Markdown

PR Compliance Guide 🔍

Below is a summary of compliance checks for this PR:

Security Compliance
CSRF delete bypass

Description: Deletion confirmation can now be satisfied via a confirmed=true query parameter, which can
enable CSRF-style or link-triggered deletion flows (e.g., a malicious site causing a
victim’s browser to send an authenticated DELETE with ?confirmed=true), depending on the
app’s auth/CSRF protections.
profile-routes.ts [378-383]

Referred Code
// Support confirmed from query param (frontend) OR body (legacy/tests)
const query = request.query as { confirmed?: string };
const bodyResult = DeleteSchema.safeParse(request.body);
const confirmedFromQuery = query.confirmed === 'true';
const confirmedFromBody = bodyResult.success ? bodyResult.data.confirmed : false;
const confirmed = confirmedFromQuery || confirmedFromBody;
Resource exhaustion risk

Description: Removing the AI worker’s internal concurrency semaphore means request-level conversion
concurrency is no longer bounded in-process, which can allow resource-exhaustion/DoS
(CPU/RAM/thread oversubscription) if the worker endpoints can be hit directly or if
queue/job concurrency is misconfigured.
pdf_converter.py [98-103]

Referred Code
async def to_markdown(
    self, file_path: str, ocr_mode: str = "auto", num_threads: int = 4
) -> ProcessorOutput:
    """Convert PDF/DOCX to Markdown."""
    return await self._convert_internal(file_path, ocr_mode, num_threads)
Ticket Compliance
🎫 No ticket provided
  • Create ticket/issue
Codebase Duplication Compliance
Codebase context is not defined

Follow the guide to enable codebase context checks.

Custom Compliance
🟢
Generic: Meaningful Naming and Self-Documenting Code

Objective: Ensure all identifiers clearly express their purpose and intent, making code
self-documenting

Status: Passed

Learn more about managing compliance generic rules or creating your own custom rules

Generic: Comprehensive Audit Trails

Objective: To create a detailed and reliable record of critical system actions for security analysis
and compliance.

Status:
Delete audit logging: The updated profile delete handler adds new confirmation input paths but does not show any
explicit audit log entry (user ID, action, outcome), so it is unclear whether this
critical delete action is captured in audit trails.

Referred Code
fastify.delete('/api/profiles/:id', async (request, reply) => {
  const { id } = request.params as { id: string };

  // Support confirmed from query param (frontend) OR body (legacy/tests)
  const query = request.query as { confirmed?: string };
  const bodyResult = DeleteSchema.safeParse(request.body);
  const confirmedFromQuery = query.confirmed === 'true';
  const confirmedFromBody = bodyResult.success ? bodyResult.data.confirmed : false;
  const confirmed = confirmedFromQuery || confirmedFromBody;

Learn more about managing compliance generic rules or creating your own custom rules

Generic: Robust Error Handling and Edge Case Management

Objective: Ensure comprehensive error handling that provides meaningful context and graceful
degradation

Status:
Silent create failures: In the create/duplicate flow, non-CONFLICT failures are only logged to console.error
without any user-facing feedback, so users may experience silent failures without
actionable guidance.

Referred Code
const handleSubmit = async (e: React.FormEvent) => {
    e.preventDefault();
    if (!formData.name.trim()) return;
    setNameError(null);

    try {
        await createMutation.mutateAsync(formData);
        onClose();
    } catch (error: unknown) {
        // Check for duplicate name error
        const err = error as Error & { code?: string };
        if (err?.code === 'CONFLICT') {
            setNameError('Profile with this name already exists');
        } else {
            console.error('Failed to create profile:', error);
        }
    }

Learn more about managing compliance generic rules or creating your own custom rules

Generic: Secure Error Handling

Objective: To prevent the leakage of sensitive system information through error messages while
providing sufficient detail for internal debugging.

Status:
Exposes server message: The client now throws new Error(errorData.message || ...) from the server response, which
may surface internal implementation details to end users if upstream UI displays
err.message.

Referred Code
const errorData = await response.json().catch(() => ({}));
const error = new Error(errorData.message || `HTTP ${response.status}`) as Error & { code?: string };
error.code = errorData.error;  // Preserve error code (e.g., 'CONFLICT')
throw error;

Learn more about managing compliance generic rules or creating your own custom rules

Generic: Secure Logging Practices

Objective: To ensure logs are useful for debugging and auditing without exposing sensitive
information like PII, PHI, or cardholder data.

Status:
Console logs raw error: The dialog logs the raw error object via console.error, which could inadvertently include
sensitive server-provided details depending on what upstream layers attach to the error.

Referred Code
} catch (error: unknown) {
    // Check for duplicate name error
    const err = error as Error & { code?: string };
    if (err?.code === 'CONFLICT') {
        setNameError('Profile with this name already exists');
    } else {
        console.error('Failed to create profile:', error);
    }

Learn more about managing compliance generic rules or creating your own custom rules

Generic: Security-First Input Validation and Data Handling

Objective: Ensure all data inputs are validated, sanitized, and handled securely to prevent
vulnerabilities

Status:
Query confirmation parsing: The delete endpoint accepts confirmed from the query string as a raw string and treats
only the literal 'true' as truthy, so it is unclear whether broader request
validation (schema validation for query params and authorization checks) is enforced
elsewhere for this external input.

Referred Code
// Support confirmed from query param (frontend) OR body (legacy/tests)
const query = request.query as { confirmed?: string };
const bodyResult = DeleteSchema.safeParse(request.body);
const confirmedFromQuery = query.confirmed === 'true';
const confirmedFromBody = bodyResult.success ? bodyResult.data.confirmed : false;
const confirmed = confirmedFromQuery || confirmedFromBody;

Learn more about managing compliance generic rules or creating your own custom rules

Compliance status legend 🟢 - Fully Compliant
🟡 - Partial Compliant
🔴 - Not Compliant
⚪ - Requires Further Human Verification
🏷️ - Compliance label

@qodo-code-review

Copy link
Copy Markdown

PR Code Suggestions ✨

Explore these optional code suggestions:

CategorySuggestion                                                                                                                                    Impact
High-level
Unify concurrency control under a single variable

The suggestion recommends renaming the PDF_CONCURRENCY environment variable to a
more generic name like PROCESSING_CONCURRENCY. This change would better reflect
its new role in controlling the entire processing pipeline's parallelism.

Examples:

.env.example [14]
PDF_CONCURRENCY=3
apps/ai-worker/src/config.py [25]
    # Note: Concurrency is controlled by BullMQ's PDF_CONCURRENCY env var

Solution Walkthrough:

Before:

# .env.example
# Infrastructure
PDF_CONCURRENCY=3

# docs/processing-settings.md
| Source | Setting | Default |
|--------|---------|---------|
| Env | `PDF_CONCURRENCY` | 1 |

**What:** Parallel document processing jobs (controls both BullMQ and AI worker).

After:

# .env.example
# Infrastructure
PROCESSING_CONCURRENCY=3

# docs/processing-settings.md
| Source | Setting | Default |
|--------|---------|---------|
| Env | `PROCESSING_CONCURRENCY` | 1 |

**What:** Parallel document processing jobs (controls both BullMQ and AI worker).
Suggestion importance[1-10]: 7

__

Why: The suggestion correctly identifies the architectural shift to a single concurrency variable and proposes a valuable renaming of PDF_CONCURRENCY to improve clarity, which is a significant enhancement for configuration and maintainability.

Medium
Possible issue
Reset error state on dialog open

Reset the nameError state to null within the useEffect hook that runs when the
dialog opens. This prevents stale error messages from being displayed when the
user reopens the form.

apps/frontend/src/components/profiles/ProfileFormDialog.tsx [60-89]

 useEffect(() => {
     if (open) {
+        setNameError(null); // Reset error state on open
         if (sourceProfile) {
             // Duplicate mode: pre-fill with source data
             setFormData({
                 name: generateDuplicateName(sourceProfile.name),
                 description: sourceProfile.description || '',
                 conversionTableRows: sourceProfile.conversionTableRows,
                 conversionTableCols: sourceProfile.conversionTableCols,
                 pdfOcrMode: sourceProfile.pdfOcrMode,
                 pdfOcrLanguages: sourceProfile.pdfOcrLanguages,
                 pdfNumThreads: sourceProfile.pdfNumThreads,
                 pdfTableStructure: sourceProfile.pdfTableStructure,
                 documentChunkSize: sourceProfile.documentChunkSize,
                 documentChunkOverlap: sourceProfile.documentChunkOverlap,
                 documentHeaderLevels: sourceProfile.documentHeaderLevels,
                 presentationMinChunk: sourceProfile.presentationMinChunk,
                 tabularRowsPerChunk: sourceProfile.tabularRowsPerChunk,
                 qualityMinChars: sourceProfile.qualityMinChars,
                 qualityMaxChars: sourceProfile.qualityMaxChars,
                 qualityPenaltyPerFlag: sourceProfile.qualityPenaltyPerFlag,
                 autoFixEnabled: sourceProfile.autoFixEnabled,
                 autoFixMaxPasses: sourceProfile.autoFixMaxPasses,
             });
         } else {
             // Create mode: use defaults
             setFormData(DEFAULT_FORM_DATA);
         }
     }
 }, [open, sourceProfile]);
  • Apply / Chat
Suggestion importance[1-10]: 7

__

Why: This suggestion correctly identifies a UI state bug where an error message persists incorrectly. Fixing it significantly improves the user experience by ensuring the form is in a clean state when opened.

Medium
Prevent shared state mutation bugs

Refactor the DEFAULT_FORM_DATA constant into a function that returns a new
object. This prevents potential shared state mutation bugs by ensuring a fresh
copy of the default data is used each time.

apps/frontend/src/components/profiles/ProfileFormDialog.tsx [28-50]

-const DEFAULT_FORM_DATA: ProfileCreateData = {
+const getDefaultFormData = (): ProfileCreateData => ({
     name: '',
     description: '',
     // Conversion defaults
     conversionTableRows: 35,
     conversionTableCols: 20,
     pdfOcrMode: 'auto',
     pdfOcrLanguages: 'en',
     pdfNumThreads: 4,
     pdfTableStructure: false,
     // Chunking defaults
     documentChunkSize: 1000,
     documentChunkOverlap: 100,
     documentHeaderLevels: 3,
     presentationMinChunk: 200,
     tabularRowsPerChunk: 20,
     // Quality defaults
     qualityMinChars: 50,
     qualityMaxChars: 2000,
     qualityPenaltyPerFlag: 0.15,
     autoFixEnabled: true,
     autoFixMaxPasses: 2,
-};
+});
  • Apply / Chat
Suggestion importance[1-10]: 6

__

Why: The suggestion correctly identifies a potential mutation bug with a shared default object and proposes a robust solution by using a factory function, improving code safety and maintainability.

Low
  • More

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant