Feat/automated high confidence autofix phase III - #112
Open
shrutu0929 wants to merge 5 commits into
Open
shrutu0929 wants to merge 5 commits into
shrutu0929 wants to merge 5 commits into
Conversation
added 4 commits
March 11, 2026 13:43
Contributor
📝 WalkthroughWalkthroughThis PR integrates AI-assisted code suggestion and triage capabilities into Refactron. It adds configuration options for AI triage, implements LLM-based batch issue evaluation, introduces an AI suggestion fixer, and enhances the code smell analyzer with optional AI-powered confidence filtering and fix generation. Supporting improvements to RAG components, type annotations, linter configuration, and CI/CD workflows are also included. Changes
Sequence DiagramsequenceDiagram
participant Code as Source Code
participant CSA as CodeSmellAnalyzer
participant LLMOrch as LLMOrchestrator
participant Engine as AutoFixEngine
participant Fixer as AISuggestionFixer
Code->>CSA: analyze(code)
CSA->>CSA: collect_issues()
CSA->>LLMOrch: evaluate_issues_batch(issues)
LLMOrch-->>CSA: confidence_scores {issue_id: score}
CSA->>CSA: filter(confidence < 0.3)
CSA->>LLMOrch: generate_suggestion(high_conf_issue)
LLMOrch-->>CSA: {suggestion, explanation, reasoning}
CSA-->>Code: enriched_issues[with AI metadata]
Code->>Engine: fix(code, issues)
Engine->>Fixer: can_fix(issue)
alt has_ai_suggestion
Fixer->>Fixer: preview(issue)
Fixer-->>Engine: FixResult {diff, fixed_code}
else rule_based_fixer
Engine-->>Engine: use rule fixer
end
Engine-->>Code: fixed_code
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested labels
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 7
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
docs/essentials/installation.mdx (1)
75-81:⚠️ Potential issue | 🟡 MinorMissing
tree-sitter-pythondependency in documentation.The AI & RAG Dependencies section is missing
tree-sitter-python, which is listed separately fromtree-sitterinpyproject.toml(line 42). Additionally,pyyamlandrequestsfrom the core dependencies in the manifest aren't documented in the Core Dependencies accordion.Consider updating for completeness:
📝 Suggested update
<Accordion title="AI & RAG Dependencies"> - **chromadb** - Vector database for RAG - **tree-sitter** - Code parsing + - **tree-sitter-python** - Python grammar for tree-sitter - **sentence-transformers** - Embeddings - **groq** - LLM integration - **pydantic** - Data validation </Accordion>🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/essentials/installation.mdx` around lines 75 - 81, Update the dependency lists in the documentation: inside the "AI & RAG Dependencies" Accordion add "tree-sitter-python" alongside "tree-sitter" (to match pyproject.toml), and in the "Core Dependencies" Accordion add "pyyaml" and "requests" to reflect the manifest's core deps; ensure the exact strings match what's used in the manifest so readers can find/install the same packages..github/workflows/auto-label.yml (1)
121-132:⚠️ Potential issue | 🟠 MajorRemove the unnecessary checkout from this pull-request-target job.
The labeling logic only needs GitHub API data (
pulls.listFiles, PR metadata). It does not read or execute any repository contents. Theactions/checkoutstep provides no functional benefit and unnecessarily widens the attack surface in a privileged context..github/workflows/first-time-contributor.ymldemonstrates the same pattern without checkout.🔒 Suggested hardening
label-pull-requests: if: github.event_name == 'pull_request_target' runs-on: ubuntu-latest steps: - - name: Checkout code - uses: actions/checkout@v6 - with: - fetch-depth: 0 - - name: Auto-label PRs uses: actions/github-script@v8🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/auto-label.yml around lines 121 - 132, Remove the unnecessary checkout step from the pull_request_target job: delete the "Checkout code" step that uses actions/checkout@v6 so the "Auto-label PRs" step (uses: actions/github-script@v8 with github-token) runs without checking out the repo; verify there are no other steps in that job that reference the workspace or require repository files before committing the change.
🧹 Nitpick comments (2)
tests/test_patterns_integration.py (1)
260-267: Collapse the repeated isolation assertion into one check.This block now asserts
refactron1.pattern_storage.storage_dir != refactron2.pattern_storage.storage_dirthree times. Keeping a single assertion here is enough and makes the test intent easier to read.♻️ Suggested cleanup
# Verify actual pattern isolation assert refactron1.pattern_storage.storage_dir != refactron2.pattern_storage.storage_dir # Note: Anonymized fingerprinting may make structurally similar code have same hash # which is correct behavior - the test should check storage isolation - assert refactron1.pattern_storage.storage_dir != refactron2.pattern_storage.storage_dir - - # Verify storage directories are separate (isolation mechanism) - assert refactron1.pattern_storage.storage_dir != refactron2.pattern_storage.storage_dir🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/test_patterns_integration.py` around lines 260 - 267, The three identical assertions comparing refactron1.pattern_storage.storage_dir and refactron2.pattern_storage.storage_dir are redundant; keep a single assertion that verifies they differ and remove the other two duplicate lines (look for the repeated refactron1.pattern_storage.storage_dir != refactron2.pattern_storage.storage_dir assertions in the test and collapse them into one).refactron/llm/backend_client.py (1)
116-116: Redundantbool()wrapper.The comparison
response.status_code == 200already evaluates to a boolean. Thebool()wrapper is unnecessary but harmless—likely added to satisfy a strict type checker.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@refactron/llm/backend_client.py` at line 116, Remove the redundant bool() wrapper around the comparison and return the boolean expression directly: replace the line "return bool(response.status_code == 200)" with "return response.status_code == 200" so the function (the line referencing response.status_code) returns the comparison result without unnecessary conversion.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@refactron/analyzers/code_smell_analyzer.py`:
- Around line 17-23: The CodeSmellAnalyzer now accepts an optional orchestrator
but the analyzer factory still calls CodeSmellAnalyzer(self.config) so
enable_ai_triage never receives the orchestrator; update the factory code that
constructs CodeSmellAnalyzer to pass the orchestrator instance (e.g.,
CodeSmellAnalyzer(self.config, orchestrator=self.orchestrator) or the local
orchestrator variable) and ensure the factory/Refactron class
exposes/initializes self.orchestrator so it is threaded into other analyzer
constructions as needed.
- Around line 73-77: The loop is using rule_id (a rule type) as the key for
confidence_scores which collapses multiple findings of the same rule; change the
lookup to use the per-instance unique key that evaluate_issues_batch returns
instead. Concretely, update the code around evaluate_issues_batch and the loop
that assigns issue_id so that issue_id = <the unique per-finding key
returned/used by evaluate_issues_batch> (e.g., issue.instance_id or the explicit
key name returned by evaluate_issues_batch) or fall back to f"issue_{i}" only if
no per-instance id exists, and then use that issue_id to index
confidence_scores; adjust evaluate_issues_batch call if needed to ensure it
returns a mapping keyed by that unique per-finding identifier.
In `@refactron/autofix/engine.py`:
- Around line 85-87: The current fixable check uses CodeIssue.suggestion (which
can be non-AI prose) causing non-AI guidance to be treated as a code-fixable
issue; update the gating so only true AI-generated suggestions are considered by
replacing checks of issue.suggestion with issue.ai_suggestion (e.g., change the
return condition to check bool(issue.ai_suggestion) and similarly update the
other occurrence around lines 106-111), ensuring that AISuggestionFixer is only
invoked when issue.ai_suggestion is present rather than when issue.suggestion
contains advisory text.
In `@refactron/llm/orchestrator.py`:
- Around line 249-265: The evaluate_issues_batch stub currently assigns a high
confidence of 1.0 for every issue, which triggers downstream immediate AI fixes;
change the neutral placeholder to a lower value (e.g., 0.5) so it does not
exceed the 0.8 auto-fix threshold used by the analyzer. In the
evaluate_issues_batch method (and when computing issue_id via getattr(issue,
"rule_id", None) or f"issue_{i}"), set scores[issue_id] = 0.5 (or another safe
<0.8 value) instead of 1.0 so the default behavior is conservative until a real
LLM-based implementation is added.
In `@refactron/rag/indexer.py`:
- Around line 284-291: The _load_metadata function should validate that the
parsed JSON is a mapping before returning it: instead of blindly returning
cast(Dict, json.load(f)), load the file inside a try/except for
json.JSONDecodeError, check isinstance(data, dict) (or Mapping) after
json.load(f), and return the dict only if it is an object; otherwise log or
silently return {} so callers like get_stats() can safely call .get(...) without
error. Ensure you still read from metadata_file / "metadata.json" and handle
missing/invalid JSON by returning an empty dict.
In `@refactron/rag/parser.py`:
- Around line 63-99: The fallback path after the Language(...) attempts still
calls the removed API Parser().set_language(py_language) (see lang_data,
py_language, Parser, Language, and parser.set_language) which will fail on
modern py-tree-sitter; change the fallback to detect supported APIs by first
trying Parser(py_language) (already present), and if that raises then create
parser = Parser() and set the language via the supported attribute assignment
parser.language = py_language (not set_language); if that assignment/errors,
raise a RuntimeError that preserves the original exception (use raise ... from
err) to provide context.
In `@scripts/analyze_feedback_data.py`:
- Around line 15-20: The script fails when run directly because imports like
from refactron.patterns.storage import PatternStorage assume the project root is
on sys.path; add a small sys.path bootstrap at the top of the module (before
imports that reference refactron) to insert the repository root (one level up
from the scripts directory) into sys.path so analyze_feedback() and any code
that imports PatternStorage can run when the file is executed directly.
---
Outside diff comments:
In @.github/workflows/auto-label.yml:
- Around line 121-132: Remove the unnecessary checkout step from the
pull_request_target job: delete the "Checkout code" step that uses
actions/checkout@v6 so the "Auto-label PRs" step (uses: actions/github-script@v8
with github-token) runs without checking out the repo; verify there are no other
steps in that job that reference the workspace or require repository files
before committing the change.
In `@docs/essentials/installation.mdx`:
- Around line 75-81: Update the dependency lists in the documentation: inside
the "AI & RAG Dependencies" Accordion add "tree-sitter-python" alongside
"tree-sitter" (to match pyproject.toml), and in the "Core Dependencies"
Accordion add "pyyaml" and "requests" to reflect the manifest's core deps;
ensure the exact strings match what's used in the manifest so readers can
find/install the same packages.
---
Nitpick comments:
In `@refactron/llm/backend_client.py`:
- Line 116: Remove the redundant bool() wrapper around the comparison and return
the boolean expression directly: replace the line "return
bool(response.status_code == 200)" with "return response.status_code == 200" so
the function (the line referencing response.status_code) returns the comparison
result without unnecessary conversion.
In `@tests/test_patterns_integration.py`:
- Around line 260-267: The three identical assertions comparing
refactron1.pattern_storage.storage_dir and
refactron2.pattern_storage.storage_dir are redundant; keep a single assertion
that verifies they differ and remove the other two duplicate lines (look for the
repeated refactron1.pattern_storage.storage_dir !=
refactron2.pattern_storage.storage_dir assertions in the test and collapse them
into one).
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 4b1f85f7-1090-4b60-8f2a-e66e3396767e
📒 Files selected for processing (54)
.flake8.github/workflows/auto-label.ymlcoverage.jsondocs/advanced/ci-cd.mdxdocs/advanced/monitoring.mdxdocs/advanced/performance.mdxdocs/api-reference/refactron-class.mdxdocs/docs.jsondocs/essentials/authentication.mdxdocs/essentials/configuration.mdxdocs/essentials/installation.mdxdocs/guides/ai-features.mdxdocs/guides/code-analysis.mdxdocs/guides/pattern-learning.mdxdocs/guides/refactoring.mdxdocs/introduction.mdxdocs/quickstart.mdxdocs/resources/faq.mdxdocumentation/docs/CLI_REFERENCE.mddocumentation/docs/api/analyzers.mddocumentation/docs/api/autofix.mddocumentation/docs/api/cicd.mddocumentation/docs/api/core.mddocumentation/docs/api/llm.mddocumentation/docs/api/patterns.mddocumentation/docs/api/rag.mddocumentation/docs/api/refactorers.mdrefactron/analyzers/code_smell_analyzer.pyrefactron/analyzers/complexity_analyzer.pyrefactron/autofix/engine.pyrefactron/autofix/fixers.pyrefactron/cli.pyrefactron/core/config.pyrefactron/core/config_loader.pyrefactron/core/memory_profiler.pyrefactron/core/repositories.pyrefactron/core/workspace.pyrefactron/llm/backend_client.pyrefactron/llm/client.pyrefactron/llm/models.pyrefactron/llm/orchestrator.pyrefactron/llm/safety.pyrefactron/rag/indexer.pyrefactron/rag/parser.pyrefactron/rag/retriever.pyscripts/analyze_feedback_data.pytests/test_ai_autofix.pytests/test_analyzers.pytests/test_backend_client.pytests/test_config_loader_edge_cases.pytests/test_config_management.pytests/test_llm_orchestrator.pytests/test_patterns_integration.pytests/test_rag_indexer.py
💤 Files with no reviewable changes (8)
- documentation/docs/api/analyzers.md
- documentation/docs/api/rag.md
- documentation/docs/api/refactorers.md
- documentation/docs/api/patterns.md
- documentation/docs/api/core.md
- documentation/docs/api/cicd.md
- documentation/docs/api/autofix.md
- documentation/docs/api/llm.md
Comment on lines
+17
to
+23
| def __init__( | ||
| self, | ||
| config: RefactronConfig, | ||
| orchestrator: Optional[LLMOrchestrator] = None, | ||
| ): | ||
| super().__init__(config) | ||
| self.orchestrator = orchestrator |
Contributor
There was a problem hiding this comment.
Wire this new constructor parameter through the real analyzer factory.
refactron/core/refactron.py:178-200 still instantiates CodeSmellAnalyzer(self.config) without an orchestrator, so enable_ai_triage never reaches the new AI branch in normal CLI/runtime usage. Right now this only works in tests that inject a mock orchestrator directly.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@refactron/analyzers/code_smell_analyzer.py` around lines 17 - 23, The
CodeSmellAnalyzer now accepts an optional orchestrator but the analyzer factory
still calls CodeSmellAnalyzer(self.config) so enable_ai_triage never receives
the orchestrator; update the factory code that constructs CodeSmellAnalyzer to
pass the orchestrator instance (e.g., CodeSmellAnalyzer(self.config,
orchestrator=self.orchestrator) or the local orchestrator variable) and ensure
the factory/Refactron class exposes/initializes self.orchestrator so it is
threaded into other analyzer constructions as needed.
Comment on lines
+73
to
+77
| for i, issue in enumerate(issues): | ||
| # evaluate_issues_batch defaults to f"issue_{i}" if rule_id is absent | ||
| issue_id = getattr(issue, "rule_id", None) or f"issue_{i}" | ||
|
|
||
| confidence = confidence_scores.get(issue_id, 1.0) |
Contributor
There was a problem hiding this comment.
rule_id is not unique enough for per-finding confidence lookup.
These IDs identify the rule type (S004, S005, etc.), not a unique finding. If one file produces multiple issues for the same rule, they all collapse onto the same batch score and triage/autofix decisions from one finding bleed into the others. Please use a per-instance key that evaluate_issues_batch() also returns.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@refactron/analyzers/code_smell_analyzer.py` around lines 73 - 77, The loop is
using rule_id (a rule type) as the key for confidence_scores which collapses
multiple findings of the same rule; change the lookup to use the per-instance
unique key that evaluate_issues_batch returns instead. Concretely, update the
code around evaluate_issues_batch and the loop that assigns issue_id so that
issue_id = <the unique per-finding key returned/used by evaluate_issues_batch>
(e.g., issue.instance_id or the explicit key name returned by
evaluate_issues_batch) or fall back to f"issue_{i}" only if no per-instance id
exists, and then use that issue_id to index confidence_scores; adjust
evaluate_issues_batch call if needed to ensure it returns a mapping keyed by
that unique per-finding identifier.
Comment on lines
+85
to
+87
| if issue.rule_id in self.fixers: | ||
| return True | ||
| return bool(issue.suggestion) |
Contributor
There was a problem hiding this comment.
Only route to ai_suggestion for actual AI-generated code.
CodeIssue.suggestion is already used by analyzers for prose guidance (for example, refactron/analyzers/code_smell_analyzer.py:122-126 stores advisory text, not replacement source). With this fallback, those issues now look “fixable”, and AISuggestionFixer can replace the whole file with that explanation string.
🧯 Safer fallback gating
def can_fix(self, issue: CodeIssue) -> bool:
@@
if issue.rule_id in self.fixers:
return True
- return bool(issue.suggestion)
+ return bool(issue.suggestion) and bool(issue.metadata.get("ai_fix_available"))
@@
- if issue.rule_id and issue.rule_id in self.fixers:
+ if issue.rule_id and issue.rule_id in self.fixers:
fixer = self.fixers[issue.rule_id]
- else:
- # Must have issue.suggestion based on can_fix() check
+ elif issue.suggestion and issue.metadata.get("ai_fix_available"):
fixer = self.fixers["ai_suggestion"]
+ else:
+ return FixResult(
+ success=False,
+ reason=f"No fixer available for issue: {issue.rule_id or 'unknown'}",
+ )Also applies to: 106-111
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@refactron/autofix/engine.py` around lines 85 - 87, The current fixable check
uses CodeIssue.suggestion (which can be non-AI prose) causing non-AI guidance to
be treated as a code-fixable issue; update the gating so only true AI-generated
suggestions are considered by replacing checks of issue.suggestion with
issue.ai_suggestion (e.g., change the return condition to check
bool(issue.ai_suggestion) and similarly update the other occurrence around lines
106-111), ensuring that AISuggestionFixer is only invoked when
issue.ai_suggestion is present rather than when issue.suggestion contains
advisory text.
Comment on lines
+249
to
+265
| def evaluate_issues_batch(self, issues: List[CodeIssue], source_code: str) -> Dict[str, float]: | ||
| """Batch evaluate confidence for multiple issues. | ||
|
|
||
| Args: | ||
| issues: List of issues to evaluate. | ||
| source_code: The source code context. | ||
|
|
||
| Returns: | ||
| A dictionary mapping issue rule_ids (or fallback IDs) to a confidence score. | ||
| """ | ||
| # Default implementation returns 1.0 (high confidence) for all issues | ||
| # Can be enhanced to actually call the LLM for batch triage | ||
| scores = {} | ||
| for i, issue in enumerate(issues): | ||
| issue_id = getattr(issue, "rule_id", None) or f"issue_{i}" | ||
| scores[issue_id] = 1.0 | ||
| return scores |
Contributor
There was a problem hiding this comment.
Use a neutral placeholder score here.
refactron/analyzers/code_smell_analyzer.py:83-95 treats confidence > 0.8 as “generate an AI fix now”. Returning 1.0 for every issue turns this stub into one extra LLM call per finding, which is a big cost/latency spike rather than a harmless default.
💡 Safer interim behavior
- def evaluate_issues_batch(self, issues: List[CodeIssue], source_code: str) -> Dict[str, float]:
+ def evaluate_issues_batch(self, issues: List[CodeIssue], _source_code: str) -> Dict[str, float]:
"""Batch evaluate confidence for multiple issues.
@@
- # Default implementation returns 1.0 (high confidence) for all issues
- # Can be enhanced to actually call the LLM for batch triage
+ # Default implementation keeps issues visible without forcing auto-fix
+ # until real batch triage is wired in.
scores = {}
for i, issue in enumerate(issues):
issue_id = getattr(issue, "rule_id", None) or f"issue_{i}"
- scores[issue_id] = 1.0
+ scores[issue_id] = 0.5
return scores🧰 Tools
🪛 Ruff (0.15.5)
[warning] 249-249: Unused method argument: source_code
(ARG002)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@refactron/llm/orchestrator.py` around lines 249 - 265, The
evaluate_issues_batch stub currently assigns a high confidence of 1.0 for every
issue, which triggers downstream immediate AI fixes; change the neutral
placeholder to a lower value (e.g., 0.5) so it does not exceed the 0.8 auto-fix
threshold used by the analyzer. In the evaluate_issues_batch method (and when
computing issue_id via getattr(issue, "rule_id", None) or f"issue_{i}"), set
scores[issue_id] = 0.5 (or another safe <0.8 value) instead of 1.0 so the
default behavior is conservative until a real LLM-based implementation is added.
Comment on lines
+284
to
+291
| def _load_metadata(self) -> Dict: | ||
| """Load index metadata.""" | ||
| metadata_file = self.index_path / "metadata.json" | ||
| if not metadata_file.exists(): | ||
| return {} | ||
|
|
||
| with open(metadata_file, "r") as f: | ||
| return json.load(f) | ||
| return cast(Dict, json.load(f)) |
Contributor
There was a problem hiding this comment.
Validate the JSON shape before returning metadata.
cast(Dict, json.load(f)) only satisfies the type checker. If metadata.json ever contains a non-object value, get_stats() will fail on .get(...) instead of falling back cleanly.
Suggested fix
-from typing import Dict, List, Optional, cast
+from typing import Any, Dict, List, Optional, cast
...
- def _load_metadata(self) -> Dict:
+ def _load_metadata(self) -> Dict[str, Any]:
"""Load index metadata."""
metadata_file = self.index_path / "metadata.json"
if not metadata_file.exists():
return {}
with open(metadata_file, "r") as f:
- return cast(Dict, json.load(f))
+ data = json.load(f)
+
+ if not isinstance(data, dict):
+ return {}
+
+ return cast(Dict[str, Any], data)📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| def _load_metadata(self) -> Dict: | |
| """Load index metadata.""" | |
| metadata_file = self.index_path / "metadata.json" | |
| if not metadata_file.exists(): | |
| return {} | |
| with open(metadata_file, "r") as f: | |
| return json.load(f) | |
| return cast(Dict, json.load(f)) | |
| def _load_metadata(self) -> Dict[str, Any]: | |
| """Load index metadata.""" | |
| metadata_file = self.index_path / "metadata.json" | |
| if not metadata_file.exists(): | |
| return {} | |
| with open(metadata_file, "r") as f: | |
| data = json.load(f) | |
| if not isinstance(data, dict): | |
| return {} | |
| return cast(Dict[str, Any], data) |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@refactron/rag/indexer.py` around lines 284 - 291, The _load_metadata function
should validate that the parsed JSON is a mapping before returning it: instead
of blindly returning cast(Dict, json.load(f)), load the file inside a try/except
for json.JSONDecodeError, check isinstance(data, dict) (or Mapping) after
json.load(f), and return the dict only if it is an object; otherwise log or
silently return {} so callers like get_stats() can safely call .get(...) without
error. Ensure you still read from metadata_file / "metadata.json" and handle
missing/invalid JSON by returning an empty dict.
Comment on lines
+63
to
+99
| lang_data = tspython.language() | ||
|
|
||
| # In some versions, tspython.language() already returns a Language object | ||
| if isinstance(lang, Language): | ||
| PY_LANGUAGE = lang | ||
| # Try to get a proper Language object | ||
| py_language = None | ||
| if isinstance(lang_data, Language): | ||
| py_language = lang_data | ||
| else: | ||
| # Try newer API first (single argument) | ||
| try: | ||
| PY_LANGUAGE = Language(lang) | ||
| except TypeError: | ||
| py_language = Language(lang_data) | ||
| except (TypeError, ValueError): | ||
| # Try older API (needs name) | ||
| try: | ||
| PY_LANGUAGE = Language(lang, "python") | ||
| except TypeError: | ||
| # Last resort: try as keyword | ||
| PY_LANGUAGE = Language(lang, name="python") | ||
|
|
||
| self.parser = Parser(PY_LANGUAGE) | ||
| py_language = Language(lang_data, "python") | ||
| except (TypeError, ValueError): | ||
| try: | ||
| py_language = Language(lang_data, name="python") | ||
| except (TypeError, ValueError): | ||
| # Fallback to using the raw data if it can be used directly | ||
| py_language = lang_data | ||
|
|
||
| # Initialize Parser - handle different tree-sitter API versions | ||
| try: | ||
| self.parser = Parser(py_language) | ||
| except Exception: | ||
| # Older API might not take it in constructor | ||
| self.parser = Parser() | ||
| try: | ||
| self.parser.set_language(py_language) | ||
| except Exception: | ||
| # If everything fails, it might be due to a mismatch between | ||
| # tree-sitter-python and tree-sitter versions. | ||
| # There's not much more we can do here but raise with context. | ||
| raise RuntimeError( | ||
| "Failed to initialize tree-sitter parser with language " | ||
| f"data of type {type(lang_data)}" | ||
| ) |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
fd "parser.py" --type f | head -5Repository: Refactron-ai/Refactron_lib
Length of output: 118
🌐 Web query:
py-tree-sitter Parser.set_language API v0.25 removal
💡 Result:
Parser.set_language(...) was removed in py-tree-sitter v0.23.0 (not v0.25.x). (github.com)
What to use instead (current 0.25.x API):
- Pass the language when constructing the parser: (tree-sitter.github.io)
from tree_sitter import Parser parser = Parser(PY_LANGUAGE)
- Or set it via the
languageattribute (the supported way to get/set the assigned language in 0.25.x): (tree-sitter.github.io)parser.language = PY_LANGUAGE
Citations:
- 1: https://github.com/tree-sitter/py-tree-sitter/releases
- 2: https://tree-sitter.github.io/py-tree-sitter/classes/tree_sitter.Parser.html
- 3: https://tree-sitter.github.io/py-tree-sitter/classes/tree_sitter.Parser.html
🏁 Script executed:
python3 << 'EOF'
# Check what's available in the current tree-sitter Python bindings
import subprocess
import sys
# First, let's see if tree-sitter is even installed
try:
import tree_sitter
print(f"tree-sitter version: {tree_sitter.__version__ if hasattr(tree_sitter, '__version__') else 'unknown'}")
# Check Parser methods
from tree_sitter import Parser
parser_methods = [m for m in dir(Parser) if not m.startswith('_')]
print(f"Parser methods: {parser_methods}")
# Check if set_language exists
if hasattr(Parser, 'set_language'):
print("✓ Parser.set_language exists")
else:
print("✗ Parser.set_language does NOT exist")
except ImportError as e:
print(f"tree-sitter not installed: {e}")
EOFRepository: Refactron-ai/Refactron_lib
Length of output: 126
🏁 Script executed:
cat -n refactron/rag/parser.py | sed -n '55,105p'Repository: Refactron-ai/Refactron_lib
Length of output: 2491
The compatibility fallback still depends on a removed py-tree-sitter API.
After the Language(...) attempts fail, lines 89–91 fall back to Parser().set_language(py_language). However, Parser.set_language() was removed in py-tree-sitter v0.23.0. The current API (v0.25.x+) uses Parser(language) in the constructor or the parser.language attribute to set the language. The fallback branch will fail on modern installs, undermining the compatibility goal. Either branch on actual version/capability support, use parser.language = py_language instead, or preserve the original exception with raise ... from err.
🧰 Tools
🪛 Ruff (0.15.5)
[warning] 87-87: Do not catch blind exception: Exception
(BLE001)
[warning] 92-92: Do not catch blind exception: Exception
(BLE001)
[warning] 96-99: Within an except clause, raise exceptions with raise ... from err or raise ... from None to distinguish them from errors in exception handling
(B904)
[warning] 96-99: Avoid specifying long messages outside the exception class
(TRY003)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@refactron/rag/parser.py` around lines 63 - 99, The fallback path after the
Language(...) attempts still calls the removed API
Parser().set_language(py_language) (see lang_data, py_language, Parser,
Language, and parser.set_language) which will fail on modern py-tree-sitter;
change the fallback to detect supported APIs by first trying Parser(py_language)
(already present), and if that raises then create parser = Parser() and set the
language via the supported attribute assignment parser.language = py_language
(not set_language); if that assignment/errors, raise a RuntimeError that
preserves the original exception (use raise ... from err) to provide context.
Comment on lines
+15
to
+20
| from typing import Dict, Optional | ||
|
|
||
| from refactron.patterns.storage import PatternStorage | ||
|
|
||
|
|
||
| def analyze_feedback(): | ||
| def analyze_feedback() -> Optional[Dict]: |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
When Python runs python scripts/analyze_feedback_data.py, what directory is inserted into sys.path[0], and will a sibling top-level package in the repo root be importable without installing the project?
💡 Result:
- Running
python scripts/analyze_feedback_data.pysetssys.path[0]to the directory containing the script, i.e.<repo-root>/scripts(typically as an absolute path). [1] - Therefore, a sibling top-level package located in
<repo-root>/(e.g.<repo-root>/my_pkg/) is not importable by default from that script unless you add the repo root toPYTHONPATH/sys.path, install the project (editable or normal), or run it in a way that puts the repo root onsys.path(commonly viapython -m ...). [2]
Sources:
[1] Python sys docs on how sys.path is initialized for python script.py (prepends the script’s directory). (docs.python.org)
[2] Python.org discussion noting it’s the script directory (not necessarily the current working directory) that is at the head of sys.path, so parent/sibling directories won’t be found automatically. (discuss.python.org)
Citations:
- 1: https://docs.python.org/3/library/sys.html?utm_source=openai
- 2: https://discuss.python.org/t/sys-path-does-not-have-current-directory-by-default/25279?utm_source=openai
🏁 Script executed:
cat scripts/analyze_feedback_data.pyRepository: Refactron-ai/Refactron_lib
Length of output: 4343
Add sys.path bootstrap to restore direct script execution.
This script has a shebang and __main__ block, making it appear directly runnable. However, python scripts/analyze_feedback_data.py adds only scripts/ to sys.path, causing the import of the sibling refactron package to fail unless the project is installed. The sys.path fix below resolves this:
Proposed fix
import json
import sys
from collections import Counter
from pathlib import Path
from typing import Dict, Optional
+if __package__ in (None, ""):
+ sys.path.insert(0, str(Path(__file__).resolve().parents[1]))
+
from refactron.patterns.storage import PatternStorage🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@scripts/analyze_feedback_data.py` around lines 15 - 20, The script fails when
run directly because imports like from refactron.patterns.storage import
PatternStorage assume the project root is on sys.path; add a small sys.path
bootstrap at the top of the module (before imports that reference refactron) to
insert the repository root (one level up from the scripts directory) into
sys.path so analyze_feedback() and any code that imports PatternStorage can run
when the file is executed directly.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
refactron/rag/parser.py (1)
102-136:⚠️ Potential issue | 🔴 CriticalRemove the stale parser bootstrap block.
This old block still executes after the new
py_languagesetup and finishes withself.parser = Parser(PY_LANGUAGE), butPY_LANGUAGE/langare not defined in the new path. Even when the new initialization succeeds, this leftover code can still crashCodeParser()before any parsing happens.🐛 Proposed direction
- PY_LANGUAGE = Language(lang, "python") - except TypeError: - # Try using the path to the compiled library (for very old or CI bindings) - try: - import os - import platform - - pkg_dir = os.path.dirname(tspython.__file__) - - # Find the correct shared library extension - system = platform.system() - if system == "Windows": - ext = ".dll" - elif system == "Darwin": - ext = ".dylib" - else: - ext = ".so" - - # Look for common names of the compiled language file - lib_path = None - for fname in os.listdir(pkg_dir): - if fname.endswith(ext): - lib_path = os.path.join(pkg_dir, fname) - break - - if lib_path: - PY_LANGUAGE = Language(lib_path, "python") - else: - # Last resort: try as keyword or whatever lang is - PY_LANGUAGE = Language(lang, name="python") - except Exception: - # Absolute last resort - PY_LANGUAGE = Language(lang, name="python") - - self.parser = Parser(PY_LANGUAGE)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@refactron/rag/parser.py` around lines 102 - 136, The stale bootstrap block that references PY_LANGUAGE and lang should be removed and replaced with the new py_language usage: delete the entire try/except block that tries TypeError fallback and sets PY_LANGUAGE, then ensure the parser is initialized with the already-established py_language by calling Parser(py_language) (i.e., replace any remaining self.parser = Parser(PY_LANGUAGE) with self.parser = Parser(py_language) or equivalent), and remove any unused imports/variables that were only for the old fallback.
♻️ Duplicate comments (1)
refactron/llm/orchestrator.py (1)
256-272:⚠️ Potential issue | 🟠 MajorThis early return disables real batch triage.
Because Line 272 returns before the existing implementation below runs, AI triage now always returns
1.0for every issue. Inrefactron/analyzers/code_smell_analyzer.py:64-105, that crosses the> 0.8threshold and forces suggestion generation for every finding instead of conservatively filtering.💡 Safer interim fix
def evaluate_issues_batch(self, issues: List[CodeIssue], source_code: str) -> Dict[str, float]: """Batch evaluate confidence for multiple issues. @@ - # Default implementation returns 1.0 (high confidence) for all issues - # Can be enhanced to actually call the LLM for batch triage + # Conservative fallback until the real implementation below is wired back in. scores = {} for i, issue in enumerate(issues): issue_id = getattr(issue, "rule_id", None) or f"issue_{i}" - scores[issue_id] = 1.0 + scores[issue_id] = 0.5 return scores🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@refactron/llm/orchestrator.py` around lines 256 - 272, The early-return placeholder in evaluate_issues_batch causes every issue to get a score of 1.0 and bypasses the real batch triage logic; remove or replace the placeholder loop that sets scores = {issue_id: 1.0} and the immediate return so the function continues to the intended implementation that performs real LLM-based batch triage (or implement actual batching there), ensuring evaluate_issues_batch (the method name) computes and returns proper confidence scores instead of always returning 1.0 for every CodeIssue.
🧹 Nitpick comments (2)
tests/test_patterns_integration.py (1)
260-278: Remove redundant duplicate assertions.The assertion
refactron1.pattern_storage.storage_dir != refactron2.pattern_storage.storage_dirappears three times (lines 261, 275, 278). A single assertion is sufficient to verify storage isolation. Additionally, this is already implied by line 253's check (storage1_dir != storage2_dir) since the configs explicitly set these paths.The
ifblock at lines 268-270 is also dead code (only containspass).♻️ Proposed cleanup
# Verify actual pattern isolation assert refactron1.pattern_storage.storage_dir != refactron2.pattern_storage.storage_dir - # Verify actual pattern isolation: patterns from project1 should not be in project2 - # and vice versa (they use different storage directories) - patterns_set1 = {p.pattern_hash for p in patterns1.values()} - patterns_set2 = {p.pattern_hash for p in patterns2.values()} - - # In most cases, these sets should be disjoint if patterns were learned - if patterns_set1 and patterns_set2: - # Basic check that isolation works at directory level (already checked above) - pass - - # Verify the key property: storage directories are different (isolation works) - # Note: Anonymized fingerprinting may make structurally similar code have same hash - # which is correct behavior - the test should check storage isolation - assert refactron1.pattern_storage.storage_dir != refactron2.pattern_storage.storage_dir - - # Verify storage directories are separate (isolation mechanism) - assert refactron1.pattern_storage.storage_dir != refactron2.pattern_storage.storage_dir🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/test_patterns_integration.py` around lines 260 - 278, Remove the redundant duplicate assertions and dead code: keep a single assertion comparing refactron1.pattern_storage.storage_dir != refactron2.pattern_storage.storage_dir (remove the two extra identical asserts) and delete the no-op if block that only contains "pass" (the patterns_set1 / patterns_set2 temporary sets can remain if used elsewhere, otherwise remove their unused creation); ensure the existing check that compares storage1_dir != storage2_dir (or the single refactron*.pattern_storage.storage_dir assertion) is the only test that verifies storage isolation.refactron/cli.py (1)
1149-1150: Remove the unreachable second return.Line 1150 can never run after the new return on Line 1149, so it just leaves dead code behind and keeps the old cast path around unnecessarily.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@refactron/cli.py` around lines 1149 - 1150, Remove the unreachable duplicate return by deleting the second line "return cast(Path, selected_path)"; keep the first "return Path(selected_path)" so the function returns a Path instance (using Path and selected_path from the surrounding code) and eliminate the dead code referencing cast to avoid confusion.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@refactron/llm/backend_client.py`:
- Around line 96-98: The code currently does return str(data["content"]) which
turns a backend null into the literal "None"; change the logic after
response.json() to explicitly handle null: call data = response.json(), then if
data.get("content") is None return an empty string (or raise an explicit error),
otherwise return the content as a str (e.g. return cast(str, data["content"])).
Update the code paths that reference data["content"] (the response.json()
handling) to remove the duplicate returns and ensure None is not converted into
"None".
In `@refactron/rag/indexer.py`:
- Around line 289-298: Remove the duplicate _load_metadata definition and
collapse the duplicated return so the function has a single valid signature and
return; specifically, keep one def _load_metadata(self) -> Dict[str, Any]:
implementation, ensure it uses self.index_path to build metadata_file, checks
metadata_file.exists(), opens and json.load(f), and returns cast(Dict[str, Any],
...). Remove the earlier duplicated def _load_metadata(self) -> Dict: and the
extra return statement so the module parses cleanly.
- Around line 188-192: The code currently mutates CodeChunk.content by
prepending "Summary: ..." which overwrites the raw source; instead, preserve
chunk.content and only store the summary in chunk.metadata["ai_summary"], and
use a separate string (e.g., embed_text = f"Summary:
{summary}\n\n{chunk.content}") when creating embeddings/index entries; update
the indexing call that currently consumes chunk.content to use that embed_text
(look for the place where chunk.content is modified in the indexer and where the
indexer passes content to the embedding/index function), leaving
CodeChunk.content untouched so retriever/LLM consumers still get the original
code.
In `@scripts/analyze_feedback_data.py`:
- Around line 24-25: There are two conflicting analyze_feedback function headers
causing a syntax error; keep a single definition with the full typed signature
(def analyze_feedback() -> Optional[Dict[str, Any]]:) and remove the duplicate
empty header so the function body that follows is attached to this one
definition; update any imports/types if needed to ensure Optional and Any are
imported and the function returns the declared type.
---
Outside diff comments:
In `@refactron/rag/parser.py`:
- Around line 102-136: The stale bootstrap block that references PY_LANGUAGE and
lang should be removed and replaced with the new py_language usage: delete the
entire try/except block that tries TypeError fallback and sets PY_LANGUAGE, then
ensure the parser is initialized with the already-established py_language by
calling Parser(py_language) (i.e., replace any remaining self.parser =
Parser(PY_LANGUAGE) with self.parser = Parser(py_language) or equivalent), and
remove any unused imports/variables that were only for the old fallback.
---
Duplicate comments:
In `@refactron/llm/orchestrator.py`:
- Around line 256-272: The early-return placeholder in evaluate_issues_batch
causes every issue to get a score of 1.0 and bypasses the real batch triage
logic; remove or replace the placeholder loop that sets scores = {issue_id: 1.0}
and the immediate return so the function continues to the intended
implementation that performs real LLM-based batch triage (or implement actual
batching there), ensuring evaluate_issues_batch (the method name) computes and
returns proper confidence scores instead of always returning 1.0 for every
CodeIssue.
---
Nitpick comments:
In `@refactron/cli.py`:
- Around line 1149-1150: Remove the unreachable duplicate return by deleting the
second line "return cast(Path, selected_path)"; keep the first "return
Path(selected_path)" so the function returns a Path instance (using Path and
selected_path from the surrounding code) and eliminate the dead code referencing
cast to avoid confusion.
In `@tests/test_patterns_integration.py`:
- Around line 260-278: Remove the redundant duplicate assertions and dead code:
keep a single assertion comparing refactron1.pattern_storage.storage_dir !=
refactron2.pattern_storage.storage_dir (remove the two extra identical asserts)
and delete the no-op if block that only contains "pass" (the patterns_set1 /
patterns_set2 temporary sets can remain if used elsewhere, otherwise remove
their unused creation); ensure the existing check that compares storage1_dir !=
storage2_dir (or the single refactron*.pattern_storage.storage_dir assertion) is
the only test that verifies storage isolation.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: d147fc2e-e532-4d06-946e-1db12ad338ef
📒 Files selected for processing (11)
refactron/cli.pyrefactron/core/repositories.pyrefactron/llm/backend_client.pyrefactron/llm/client.pyrefactron/llm/orchestrator.pyrefactron/llm/safety.pyrefactron/rag/indexer.pyrefactron/rag/parser.pyrefactron/rag/retriever.pyscripts/analyze_feedback_data.pytests/test_patterns_integration.py
🚧 Files skipped from review as they are similar to previous changes (3)
- refactron/core/repositories.py
- refactron/llm/safety.py
- refactron/rag/retriever.py
Comment on lines
96
to
98
| data = response.json() | ||
| return str(data["content"]) | ||
| return cast(str, data["content"]) |
Contributor
There was a problem hiding this comment.
Avoid turning null content into the literal string "None".
str(data["content"]) makes a backend null look like a successful completion containing "None", which is harder to detect downstream than an empty string or explicit error.
💡 Proposed fix
data = response.json()
- return str(data["content"])
- return cast(str, data["content"])
+ return str(data.get("content") or "")📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| data = response.json() | |
| return str(data["content"]) | |
| return cast(str, data["content"]) | |
| data = response.json() | |
| return str(data.get("content") or "") |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@refactron/llm/backend_client.py` around lines 96 - 98, The code currently
does return str(data["content"]) which turns a backend null into the literal
"None"; change the logic after response.json() to explicitly handle null: call
data = response.json(), then if data.get("content") is None return an empty
string (or raise an explicit error), otherwise return the content as a str (e.g.
return cast(str, data["content"])). Update the code paths that reference
data["content"] (the response.json() handling) to remove the duplicate returns
and ensure None is not converted into "None".
Comment on lines
+188
to
192
| # Prepend summary to content for embedding | ||
| # (makes it searchable by plain English) | ||
| # Prepend summary for semantic searchability | ||
| chunk.content = f"Summary: {summary}\n\n{chunk.content}" | ||
| chunk.metadata["ai_summary"] = summary |
Contributor
There was a problem hiding this comment.
Don't overwrite the stored code with the AI summary.
This mutates CodeChunk.content before indexing, so retrieval now returns "Summary: ..." plus code instead of raw code. refactron/rag/retriever.py:22-33 passes that content through verbatim, which means rag search and LLM context consumers stop receiving valid source snippets.
💡 Safer approach
- if summary:
- # Prepend summary to content for embedding
- # (makes it searchable by plain English)
- chunk.content = f"Summary: {summary}\n\n{chunk.content}"
- chunk.metadata["ai_summary"] = summary
+ if summary:
+ chunk.metadata["ai_summary"] = summary🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@refactron/rag/indexer.py` around lines 188 - 192, The code currently mutates
CodeChunk.content by prepending "Summary: ..." which overwrites the raw source;
instead, preserve chunk.content and only store the summary in
chunk.metadata["ai_summary"], and use a separate string (e.g., embed_text =
f"Summary: {summary}\n\n{chunk.content}") when creating embeddings/index
entries; update the indexing call that currently consumes chunk.content to use
that embed_text (look for the place where chunk.content is modified in the
indexer and where the indexer passes content to the embedding/index function),
leaving CodeChunk.content untouched so retriever/LLM consumers still get the
original code.
Comment on lines
+289
to
298
| def _load_metadata(self) -> Dict: | ||
| def _load_metadata(self) -> Dict[str, Any]: | ||
| """Load index metadata.""" | ||
| metadata_file = self.index_path / "metadata.json" | ||
| if not metadata_file.exists(): | ||
| return {} | ||
|
|
||
| with open(metadata_file, "r") as f: | ||
| return cast(Dict, json.load(f)) | ||
| return cast(Dict[str, Any], json.load(f)) |
Contributor
There was a problem hiding this comment.
_load_metadata() is duplicated and breaks the module.
Line 289 starts one function definition and Line 290 immediately starts another, so the file will not parse. The duplicated return on Lines 297-298 should also be collapsed once the signature is fixed.
🐛 Proposed fix
- def _load_metadata(self) -> Dict:
def _load_metadata(self) -> Dict[str, Any]:
"""Load index metadata."""
metadata_file = self.index_path / "metadata.json"
if not metadata_file.exists():
return {}
with open(metadata_file, "r") as f:
- return cast(Dict, json.load(f))
return cast(Dict[str, Any], json.load(f))🧰 Tools
🪛 Ruff (0.15.5)
[warning] 290-290: Expected an indented block after function definition
(invalid-syntax)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@refactron/rag/indexer.py` around lines 289 - 298, Remove the duplicate
_load_metadata definition and collapse the duplicated return so the function has
a single valid signature and return; specifically, keep one def
_load_metadata(self) -> Dict[str, Any]: implementation, ensure it uses
self.index_path to build metadata_file, checks metadata_file.exists(), opens and
json.load(f), and returns cast(Dict[str, Any], ...). Remove the earlier
duplicated def _load_metadata(self) -> Dict: and the extra return statement so
the module parses cleanly.
Comment on lines
+24
to
25
| def analyze_feedback() -> Optional[Dict]: | ||
| def analyze_feedback() -> Optional[Dict[str, Any]]: |
Contributor
There was a problem hiding this comment.
Collapse this to a single function definition.
The first def analyze_feedback(...) has no body because Line 25 immediately starts a second definition, so this file will raise a syntax error on import/run.
🐛 Proposed fix
-def analyze_feedback() -> Optional[Dict]:
def analyze_feedback() -> Optional[Dict[str, Any]]:📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| def analyze_feedback() -> Optional[Dict]: | |
| def analyze_feedback() -> Optional[Dict[str, Any]]: | |
| def analyze_feedback() -> Optional[Dict[str, Any]]: |
🧰 Tools
🪛 Ruff (0.15.5)
[warning] 25-25: Expected an indented block after function definition
(invalid-syntax)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@scripts/analyze_feedback_data.py` around lines 24 - 25, There are two
conflicting analyze_feedback function headers causing a syntax error; keep a
single definition with the full typed signature (def analyze_feedback() ->
Optional[Dict[str, Any]]:) and remove the duplicate empty header so the function
body that follows is attached to this one definition; update any imports/types
if needed to ensure Optional and Any are imported and the function returns the
declared type.
This branch has not been deployed
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
2 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.
Pull Request
📋 Description
A clear and concise description of what this PR does.
🔗 Related Issue
Closes #(issue number)
🧪 Type of Change
🧪 Testing
📝 Changes Made
🎯 Code Quality
📚 Documentation
🔒 Security
🚀 Performance
📋 Checklist
🎨 Screenshots (if applicable)
Add screenshots to help explain your changes.
📋 Additional Notes
Add any other context about the PR here.
🔍 Reviewers
@omsherikar - Please review this PR
🏷️ Labels
Please add appropriate labels to this PR:
bug- Bug fixenhancement- New featuredocumentation- Documentation updatedependencies- Dependency updatesecurity- Security updateperformance- Performance improvementrefactoring- Code refactoringtesting- Test updatesbreaking-change- Breaking changegood-first-issue- Good for new contributorshelp-wanted- Help neededpriority-high- High prioritypriority-medium- Medium prioritypriority-low- Low prioritySummary by CodeRabbit
New Features
Bug Fixes
Tests
Chores