feat(typescript-beta): strengthen typescript grammar detection and in…

[JayGhiya]

…clude next js endpoint detection as a starting example

[huly-for-github]

Connected to Huly®: UNOPL-1411

[claude]

Claude finished @JayGhiya's task —— View job

---

Code Review Complete ✅

I've completed a thorough review of this TypeScript framework detection implementation. Here's my comprehensive feedback:

Review Checklist:

• Analyze TypeScript detection architecture and implementation
• Review Next.js framework definition and extraction logic
• Examine test coverage and quality
• Check for security concerns
• Evaluate performance implications
• Review code quality and best practices
• Provide comprehensive feedback

---

🟢 Strengths

Architecture & Design

• Excellent separation of concerns: Clean division between detection service, tree-sitter detector, query builder, and source context
• Smart context reuse: TypeScriptSourceContext prevents double-parsing when the same source is processed multiple times
• Robust query caching: Hash-based caching in TypeScriptFrameworkQueryBuilder will improve performance
• Template-based queries: Dynamic query generation using .scm templates is flexible and maintainable

Implementation Quality

• Comprehensive import handling: Supports named, aliased, default, and type-only imports with proper alias resolution
• Strong error handling: Appropriate exception handling with detailed logging throughout
• Consistent patterns: Follows the same architectural patterns as the existing Python implementation
• Performance optimizations: LRU cache for import queries, query result caching

Test Coverage

• Excellent test scenarios: Covers positive cases, negative cases with missing imports, aliased imports, and edge cases
• Clear test structure: Well-organized test functions with descriptive names
• Good coverage of import gating logic: Verifies that framework detection requires appropriate imports

---

🟡 Areas for Improvement

Error Handling & Robustness

1. Broad exception handling in typescript_framework_detection_service.py:115-117:

   except Exception as e:
       logger.error("Error in TypeScript framework detection: {}", e)
       return []

   Recommendation: Consider catching specific exceptions (e.g., tree_sitter.TreeSitterError, DatabaseError) for better error categorization and handling.

2. Query parsing errors in TypeScriptFrameworkQueryBuilder.build_query() lack explicit handling:

   _QUERY_CACHE[cache_key] = tree_sitter.Query(self._language, query_source)

   Recommendation: Add try-catch around query compilation with meaningful error messages.

Code Quality & Maintainability

3. Regex escaping function in typescript_framework_query_builder.py:26-27:

   def _escape_query_regex(regex: str) -> str:
       return regex.replace("\\", "\\\\").replace('"', '\\"')

   Concern: This manual escaping might miss edge cases. Consider using re.escape() or a more robust escaping library.

4. Import alias collision handling: No explicit handling for cases where multiple imports map to the same alias name, which could cause confusion.

Performance Considerations

5. Import path expansion in both services has identical logic:

   def _expand_import_paths(import_paths: List[str]) -> List[str]:

   Recommendation: Extract to a shared utility module to avoid code duplication.

6. Source context creation: Multiple tree-sitter parses could be expensive for large files. Consider adding size limits or async processing.

---

🟢 Security Assessment

• No direct security vulnerabilities found
• Proper input sanitization: User input is safely handled through tree-sitter parsing
• No SQL injection risks: Uses parameterized queries through the ORM
• Safe file handling: Uses proper encoding handling with error recovery

---

🟢 Framework Definition Quality

The Next.js framework definition in nextjs.json is well-structured:

• Appropriate confidence score (0.95) for HTTP method detection
• Good import gating prevents false positives by requiring next/server imports
• Comprehensive HTTP method coverage (GET, POST, PUT, DELETE, PATCH, HEAD, OPTIONS)

---

🔧 Specific Recommendations

High Priority

1. Add explicit query compilation error handling:

   try:
       _QUERY_CACHE[cache_key] = tree_sitter.Query(self._language, query_source)
   except Exception as e:
       raise ValueError(f"Failed to compile TypeScript query: {e}") from e

2. Extract shared _expand_import_paths utility:

   # In src/code_confluence_flow_bridge/engine/utils/import_utils.py
   def expand_import_paths(import_paths: List[str]) -> List[str]:

Medium Priority

3. Add input validation for large source files
4. Enhance regex escaping using re.escape() or a dedicated library
5. Add metrics/telemetry for detection performance monitoring

Low Priority

6. Consider adding support for .tsx files in future iterations
7. Add validation for framework definition schemas

---

📊 Overall Assessment

This is a high-quality implementation that demonstrates:

• Strong architectural design following established patterns
• Comprehensive test coverage with good edge case handling
• Robust error handling and logging
• No security vulnerabilities
• Good performance characteristics with caching strategies

The code is production-ready with the minor improvements noted above. The TypeScript framework detection capability will be a valuable addition to the codebase.

Recommendation: ✅ Approve with suggested improvements to be addressed in follow-up work.

---

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 0489d94e0d

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

[chatgpt-codex-connector]

Capture export name separately from declaration name

The query binds @export_name and @function_name to the exact same identifier node, so construct_query.export_name_regex cannot actually filter an exported symbol independently of the declaration name. This breaks the schema contract for cases like function handler() {} with export { handler as GET }, where export-name matching should work but will never be detected with the current pattern.

Useful? React with 👍 / 👎.

[chatgpt-codex-connector]

Restrict Next.js handler detection to route.ts files

Framework detection is invoked for every .ts file without any file-convention gate, but the new Next.js feature definition describes App Router handlers in route.ts; this means any non-route module that imports next/server and exports GET/POST can be misclassified as an HTTP endpoint startpoint. That creates false positives in downstream framework usage data unless detection is constrained by filename/path (or equivalent context) before calling the detector.

Useful? React with 👍 / 👎.