Skip to content

fix(parser): Refactor parser to focus on file insertion - #919

Merged
JayGhiya merged 1 commit into
devfrom
update-commons
Oct 23, 2025
Merged

fix(parser): Refactor parser to focus on file insertion#919
JayGhiya merged 1 commit into
devfrom
update-commons

Conversation

@JayGhiya

@JayGhiya JayGhiya commented Oct 23, 2025

Copy link
Copy Markdown
Member

User description

Modify test suite to validate file-level parsing without package
tracking. Remove package-related assertions and simplify file
verification process. Update test to check file node creation,
checksums, structural signatures, and codebase relationships.


PR Type

Enhancement, Bug fix


Description

  • Remove package-level abstraction from parser, simplify to file-centric model

  • Connect files directly to codebase instead of through package hierarchy

  • Fix decorator line range detection to capture full function span

  • Update deletion logic to remove package-related operations

  • Refactor language processor to accept flat file list instead of package map


Diagram Walkthrough

flowchart LR
  A["Codebase Discovery"] -->|discover_source_files| B["Flat File List"]
  B -->|process_files_managed| C["File Nodes"]
  C -->|CONTAINS_FILE| D["Codebase Node"]
  D -->|PART_OF_CODEBASE| C
  E["Old: Package Hierarchy"] -.->|removed| F["Package Nodes"]
  F -.->|removed| G["Package Relationships"]
Loading

File Walkthrough

Relevant files
Bug fix
1 files
simplified_python_detector.py
Fix decorator line range and rename object field                 
+9/-2     
Enhancement
4 files
generic_codebase_parser.py
Remove package hierarchy, simplify to file-centric model 
+45/-394
base.py
Update iter_files to accept flat file list                             
+11/-13 
code_confluence_graph_deletion.py
Remove package deletion logic and relationships                   
+36/-351
generic_codebase_processing_activity.py
Remove packages_created metric from logging                           
+9/-8     
Tests
4 files
test_delete_repository.py
Update deletion tests to remove package assertions             
+11/-5   
test_framework_detection_with_postgres.py
Add debug logging for detection line numbers                         
+84/-9   
test_generic_codebase_parser.py
Refactor parser test to validate file-codebase relationships
+39/-112
graph_assertions.py
Remove package-related assertions and validation                 
+6/-13   
Documentation
1 files
index.html
Update page title to Unoplat Code Confluence                         
+1/-1     
Formatting
1 files
CodebaseForm.tsx
Fix trailing whitespace formatting                                             
+1/-1     
Dependencies
2 files
pyproject.toml
Update commons dependency and fix formatting                         
+3/-4     
pyproject.toml
Update commons dependency version                                               
+1/-1     
Configuration changes
1 files
example_config.json
Update root_packages configuration path                                   
+4/-1     

Modify test suite to validate file-level parsing without package
tracking. Remove package-related assertions and simplify file
verification process. Update test to check file node creation,
checksums, structural signatures, and codebase relationships.
@huly-for-github

Copy link
Copy Markdown

Connected to Huly®: UNOPL-1063

@claude

claude Bot commented Oct 23, 2025

Copy link
Copy Markdown

Claude finished @JayGhiya's task —— View job


Code Review for PR #919: Refactor parser to focus on file insertion

Todo List:

  • Analyze core parser refactoring changes
  • Review Python detector fixes
  • Evaluate test coverage and assertions
  • Check database deletion logic changes
  • Assess performance and security implications
  • Provide comprehensive feedback

📝 Review Summary

This is a major architectural refactoring that removes the package-level abstraction and simplifies the system to a file-centric model. The changes significantly reduce complexity while maintaining functionality.

✅ Strengths

Architecture Simplification

  • Excellent decision to remove package hierarchy - this eliminates unnecessary complexity
  • Clean migration from nested package structure to flat file-to-codebase relationships
  • Reduces code by 923 lines while maintaining essential functionality
  • Direct CONTAINS_FILE and PART_OF_CODEBASE relationships are more intuitive

Well-Executed Refactoring

  • Comprehensive test updates - all package-related assertions properly removed
  • Consistent changes across parser, deletion logic, and test suite
  • Proper error handling maintained throughout the refactoring
  • Database schema aligned with new simplified model

Code Quality Improvements

  • Better separation of concerns in GenericCodebaseParser
  • Cleaner transaction handling in managed sessions
  • Improved test coverage with more focused file-level assertions

⚠️ Issues Found

Critical: Decorator Range Detection Bug

Location: simplified_python_detector.py:169-170

start_line=func.start_line,
end_line=func.end_line,  # Capture from decorator to end of function

Problem: The fix description mentions "Fix decorator line range detection to capture full function span", but the implementation still uses func.start_line which points to the function definition, not the decorator line.

Impact:

  • Decorator detections will have incorrect line ranges
  • May cause issues with IDE navigation and code highlighting
  • Inconsistent behavior compared to other detection types

Recommendation:

# Need to parse decorator position from func.signature
decorator_start_line = self._extract_decorator_line(func.signature, match.group(0))
start_line=decorator_start_line or func.start_line,

Potential Performance Concern

Location: generic_codebase_parser.py:396-397

async for file_data in self.language_processor.iter_files(file_paths):
    await self.insert_files_managed(session, [file_data])

Problem: Processing files one-by-one instead of batching
Impact: Could be inefficient for large codebases due to transaction overhead
Recommendation: Consider batching files in chunks (e.g., 10-50 files per transaction)

Test Coverage Gap

Location: test_generic_codebase_parser.py:143

assert package_count == 0, f"Expected 0 package nodes, found {package_count}"

Problem: Test only verifies package nodes don't exist, but doesn't validate the new file-codebase relationships are working correctly with framework features
Recommendation: Add specific tests for framework feature relationships in the new file-centric model

🔒 Security Assessment

✅ Secure: No security concerns identified

  • Proper parameterized queries prevent SQL injection
  • No sensitive data exposure in logs
  • Transaction isolation maintained

🚀 Performance Analysis

Positive Changes:

  • Fewer graph traversals (no package hierarchy)
  • Simplified relationship management
  • Reduced memory footprint

Potential Optimizations:

  • File processing could benefit from batching
  • Consider connection pooling improvements for high-volume scenarios

📊 Test Coverage Analysis

Strengths:

  • Comprehensive validation of file nodes and properties
  • Good structural signature testing
  • Proper cleanup verification

Improvements Needed:

  • Framework feature relationship testing in new model
  • Edge case testing for large file batches
  • Performance testing for the new architecture

🎯 Recommendations

High Priority

  1. Fix decorator line range detection - This is a functional bug that should be addressed
  2. Add framework feature relationship tests - Ensure new architecture works correctly

Medium Priority

  1. Implement file batching - For better performance with large codebases
  2. Add integration tests - For the new file-centric workflow end-to-end

Low Priority

  1. Documentation updates - Update architecture docs to reflect package removal
  2. Metrics adjustment - Update logging to reflect new simplified metrics

🏆 Overall Assessment

Verdict: ✅ Approve with Minor Changes

This is a well-executed major refactoring that successfully simplifies the architecture while maintaining functionality. The decorator bug is the only critical issue that needs fixing before merge.

Risk Level: Low - Changes are well-tested and maintain backwards compatibility at the API level

Merge Recommendation: Approve after fixing the decorator line range issue


| Branch: update-commons

@qodo-code-review

Copy link
Copy Markdown

PR Compliance Guide 🔍

Below is a summary of compliance checks for this PR:

Security Compliance
🟢
No security concerns identified No security vulnerabilities detected by AI analysis. Human verification advised for critical code.
Ticket Compliance
🎫 No ticket provided
- [ ] Create ticket/issue <!-- /create_ticket --create_ticket=true -->

</details></td></tr>
Codebase Duplication Compliance
Codebase context is not defined

Follow the guide to enable codebase context checks.

Custom Compliance
No custom compliance provided

Follow the guide to enable custom compliance check.

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
Refactor simplifies ingestion but loses directory structure

The PR removes the package hierarchy, losing directory structure information in
the graph. To retain this valuable context for analysis, consider adding a
relative_path property to CodeConfluenceFile nodes.

Examples:

unoplat-code-confluence-ingestion/code-confluence-flow-bridge/src/code_confluence_flow_bridge/parser/generic_codebase_parser.py [348-373]
    async def process_and_insert_codebase(self) -> None:
        """
        Main processing method with managed transactions from shared session pool.
        Uses managed transactions to prevent concurrent access conflicts.
        """
        try:
            logger.info(f"Starting codebase processing: {self.codebase_name}")

            # 1. Discover all source files within the codebase
            file_paths = self.discover_source_files()

 ... (clipped 16 lines)
unoplat-code-confluence-ingestion/code-confluence-flow-bridge/tests/parser/test_generic_codebase_parser.py [138-143]
        # 1. Confirm that no CodeConfluencePackage nodes are written
        result, _ = neo4j_client.cypher_query(
            "MATCH (p:CodeConfluencePackage) RETURN count(p) AS count"
        )
        package_count = result[0][0] if result else 0
        assert package_count == 0, f"Expected 0 package nodes, found {package_count}"

Solution Walkthrough:

Before:

// In generic_codebase_parser.py
class GenericCodebaseParser:
    async def process_and_insert_codebase(self):
        // 1. Discover packages and files into a hierarchy
        package_files = self.discover_packages()

        // 2. Create CodeConfluencePackage nodes
        await self.create_packages_managed(...)

        // 3. Create package hierarchy relationships
        hierarchy = self.build_package_hierarchy(package_files)
        await self.create_package_hierarchy_managed(...)

        // 4. Process files and connect them to their respective package nodes
        await self.process_files_managed(session, package_files)
        // Inside insert_files_managed:
        // MERGE (p:CodeConfluencePackage)-[:CONTAINS_FILE]->(f:CodeConfluenceFile)

After:

// In generic_codebase_parser.py
class GenericCodebaseParser:
    async def process_and_insert_codebase(self):
        // 1. Discover a flat list of source files
        file_paths = self.discover_source_files()

        // 2. Process files and connect them directly to the codebase node
        await self.process_files_managed(session, file_paths)

    async def insert_files_managed(self, session, file_data_list):
        for unoplat_file in file_data_list:
            // Add relative_path to the file node properties
            file_dict = {
                "file_path": unoplat_file.file_path,
                "relative_path": calculate_relative_path(unoplat_file.file_path, self.codebase_path),
                ...
            }
            // MERGE (c:CodeConfluenceCodebase)-[:CONTAINS_FILE]->(f:CodeConfluenceFile)
Suggestion importance[1-10]: 8

__

Why: The suggestion correctly identifies a major design trade-off in the PR—the loss of directory structure information by removing package nodes—and proposes a practical solution (relative_path property) that preserves this valuable context without reintroducing the complexity the PR aims to remove.

Medium
General
Batch file insertions for performance

Improve performance by modifying process_files_managed to batch file data and
insert multiple files in a single transaction, rather than inserting them one by
one.

unoplat-code-confluence-ingestion/code-confluence-flow-bridge/src/code_confluence_flow_bridge/parser/generic_codebase_parser.py [385-405]

 async def process_files_managed(
-    self, session: AsyncSession, file_paths: List[str]
+    self, session: AsyncSession, file_paths: List[str], batch_size: int = 100
 ) -> None:
     """
-    Process all files sequentially using managed transactions.
+    Process all files and insert them in batches using managed transactions.
 
     Args:
-        session: Neo4j async session from connection pool
-        file_paths: List of file paths to process
+        session: Neo4j async session from connection pool.
+        file_paths: List of file paths to process.
+        batch_size: The number of files to include in each database write batch.
     """
     try:
+        file_batch: List[UnoplatFile] = []
         async for file_data in self.language_processor.iter_files(file_paths):
-            await self.insert_files_managed(session, [file_data])
+            file_batch.append(file_data)
+            if len(file_batch) >= batch_size:
+                await self.insert_files_managed(session, file_batch)
+                file_batch = []
+        
+        # Insert any remaining files in the last batch
+        if file_batch:
+            await self.insert_files_managed(session, file_batch)
 
         logger.info(
             f"Processed {self.files_processed} files using managed transactions"
         )
 
     except Exception as e:
         logger.error(f"Failed to process files with managed transactions: {e}")
         raise
  • Apply / Chat
Suggestion importance[1-10]: 8

__

Why: The suggestion correctly identifies a significant performance bottleneck where files are inserted one by one, each in its own transaction. Introducing batch insertion is a critical optimization that will substantially improve the performance of codebase processing for larger projects.

Medium
Combine relationship deletion queries

Combine the separate Cypher queries for deleting PART_OF_CODEBASE and
CONTAINS_FILE relationships into a single, atomic query to improve efficiency
and robustness.

unoplat-code-confluence-ingestion/code-confluence-flow-bridge/src/code_confluence_flow_bridge/processor/db/graph_db/code_confluence_graph_deletion.py [199-219]

-# Delete PART_OF_CODEBASE relationships (File → Codebase)
-part_of_codebase_query = """
-MATCH (f:CodeConfluenceFile)-[r:PART_OF_CODEBASE]->(c:CodeConfluenceCodebase)
+# Delete both PART_OF_CODEBASE and CONTAINS_FILE relationships in one query
+delete_file_rels_query = """
+MATCH (c:CodeConfluenceCodebase)-[r1:CONTAINS_FILE]->(f:CodeConfluenceFile)
 WHERE f.file_path IN $file_paths
-DELETE r
-RETURN count(r) as count
+WITH c, f, r1
+MATCH (f)-[r2:PART_OF_CODEBASE]->(c)
+DELETE r1, r2
+RETURN count(r1) + count(r2) as count
 """
-part_of_codebase_result = await tx.run(part_of_codebase_query, {"file_paths": file_paths})
-part_of_codebase_record = await part_of_codebase_result.single()
-stats["part_of_codebase_deleted"] = part_of_codebase_record["count"] if part_of_codebase_record else 0
+delete_file_rels_result = await tx.run(delete_file_rels_query, {"file_paths": file_paths})
+delete_file_rels_record = await delete_file_rels_result.single()
+# For logging purposes, we can approximate the split or just log the total.
+# Assuming relationships are always paired, we can divide by 2.
+total_deleted = delete_file_rels_record["count"] if delete_file_rels_record else 0
+stats["part_of_codebase_deleted"] = total_deleted // 2
+stats["contains_file_deleted"] = total_deleted - (total_deleted // 2)
 
-# Delete CONTAINS_FILE relationships (Codebase → File)
-contains_file_query = """
-MATCH (c:CodeConfluenceCodebase)-[r:CONTAINS_FILE]->(f:CodeConfluenceFile)
-WHERE f.file_path IN $file_paths
-DELETE r
-RETURN count(r) as count
-"""
-contains_file_result = await tx.run(contains_file_query, {"file_paths": file_paths})
-contains_file_record = await contains_file_result.single()
-stats["contains_file_deleted"] = contains_file_record["count"] if contains_file_record else 0
-
  • Apply / Chat
Suggestion importance[1-10]: 7

__

Why: The suggestion correctly points out that two separate queries are used to delete related relationships. Combining them into a single, atomic Cypher query is a valid improvement for both performance and data consistency, making the deletion process more robust.

Medium
Possible issue
Use correct decorator start line

To improve accuracy, set the start_line for decorator detections using
func.decorator_start_line instead of func.start_line, ensuring it points to the
decorator itself rather than the function definition.

unoplat-code-confluence-ingestion/code-confluence-flow-bridge/src/code_confluence_flow_bridge/engine/python/simplified_python_detector.py [165-177]

 detection = AnnotationLikeInfo(
     feature_key=spec.feature_key,
     library=spec.library,
     match_text=full_decorator,
-    start_line=func.start_line,
+    start_line=func.decorator_start_line,
     end_line=func.end_line,  # Capture from decorator to end of function
     bound_object=obj_path,
     annotation_name=method_name,
     metadata={
         "concept": "AnnotationLike",
         "source": "structural_signature",
     },
 )

[To ensure code accuracy, apply this suggestion manually]

Suggestion importance[1-10]: 6

__

Why: The suggestion correctly identifies that using func.start_line can be inaccurate for decorators. Proposing func.decorator_start_line would improve the precision of the feature detection's location, which is a valuable data quality improvement.

Low
  • More

@JayGhiya
JayGhiya merged commit c0bd597 into dev Oct 23, 2025
6 checks passed
JayGhiya added a commit that referenced this pull request Apr 8, 2026
fix(parser): Refactor parser to focus on file insertion
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