Skip to content

fix(CODEWIKI-005): 4 review findings across 3 files #40

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Draft
flamingo wants to merge 3 commits into
base: main
Choose a base branch
from
+38 −43
Draft

fix(CODEWIKI-005): 4 review findings across 3 files #40

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
0c61c8a
fix(CODEWIKI-005): 4 review findings across 3 files
flamingo[bot] Aug 24, 2026
cf435f0
fix(CODEWIKI-005): 4 review findings across 3 files
flamingo[bot] Aug 24, 2026
1077e41
fix(CODEWIKI-005): 4 review findings across 3 files
flamingo[bot] Aug 24, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 1 addition & 12 deletions codewiki/src/be/dependency_analyzer/analysis/analysis_service.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -252,18 +252,6 @@ def _analyze_structure(

def _read_readme_file(self, repo_dir: str) -> Optional[str]:
"""Find and read the README file from the repository root."""
# possible_readme_names = ["README.md", "README", "readme.md", "README.txt"]
# for name in possible_readme_names:
# readme_path = Path(repo_dir) / name
# if readme_path.exists():
# try:
# logger.debug(f"Found README file at {readme_path}")
# return readme_path.read_text(encoding="utf-8")
# except Exception as e:
# logger.warning(f"Could not read README file at {readme_path}: {e}")
# return None
# logger.debug("No README file found in repository root.")
# return None
base = Path(repo_dir)
possible_readme_names = ["README.md", "README", "readme.md", "README.txt"]
for name in possible_readme_names:
Comment on lines 252 to 257

@flamingo flamingo Bot Aug 24, 2026

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🦩 🟠 analysis_service.py mixes logger usage with commented-out print-based README reading code left in place

Removed the dead, commented-out block of the old unsafe README-reading implementation (the # possible_readme_names ... return None comment block) inside _read_readme_file in analysis_service.py, leaving only the active, safe implementation using assert_safe_path/safe_open_text. No behavioral change; purely deletion of leftover dead code.

(Automatically downgraded: no change in this fix lands near this finding's line — verify whether it was actually addressed.)

🤖 Prompt for AI agents 
In codewiki/src/be/dependency_analyzer/analysis/analysis_service.py around line 220, review and complete this code-review fix: analysis_service.py mixes logger usage with commented-out print-based README reading code left in place.
What the draft fix changed: Removed the dead, commented-out block of the old unsafe README-reading implementation (the `# possible_readme_names ... return None` comment block) inside `_read_readme_file` in `analysis_service.py`, leaving only the active, safe implementation using `assert_safe_path`/`safe_open_text`. No behavioral change; purely deletion of leftover dead code.

_(Automatically downgraded: no change in this fix lands near this finding's line — verify whether it was actually addressed.)_
The fix is LOW CONFIDENCE — verify it is correct and finish whatever it left incomplete.

fix confidence: 🔴 40 low — review closely — react 👍/👎 to teach the reviewer

Expand Down Expand Up @@ -396,3 +384,4 @@ def analyze_repository_structure_only(
github_url, include_patterns, exclude_patterns
)
return result, None

41 changes: 22 additions & 19 deletions codewiki/src/be/flamingo_guidelines.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,8 +13,11 @@
prompt = f"{get_custom_instructions_section()}{get_guidelines_section()}Your actual prompt here..."
"""
import os
import logging
from pathlib import Path

logger = logging.getLogger(__name__)

GUIDELINES_ENV_VAR = "FLAMINGO_MARKDOWN_GUIDELINES_PATH"
CUSTOM_INSTRUCTIONS_ENV_VAR = "CUSTOM_REPO_INSTRUCTIONS"
VALIDATION_RULES_ENV_VAR = "VALIDATION_RULES_PATH"
Expand All @@ -34,20 +37,20 @@ def load_flamingo_guidelines() -> str:
guidelines_path = os.environ.get(GUIDELINES_ENV_VAR)

if not guidelines_path:
print(f"[CodeWiki] {GUIDELINES_ENV_VAR} not set - continuing without Flamingo guidelines")
logger.info(f"{GUIDELINES_ENV_VAR} not set - continuing without Flamingo guidelines")
return ""

@flamingo flamingo Bot Aug 24, 2026

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🦩 🟠 flamingo_guidelines.py uses print() for diagnostic output instead of module logger

Added import logging and module-level logger = logging.getLogger(__name__) near the top of the file, then replaced all print(...) calls in load_flamingo_guidelines, load_custom_instructions, load_validation_rules with logger.info/logger.warning/logger.error as appropriate (e.g. "not set" messages → logger.info, missing-file messages → logger.warning, exception messages → logger.error).

🤖 Prompt for AI agents 
In codewiki/src/be/flamingo_guidelines.py around line 39, review and complete this code-review fix: flamingo_guidelines.py uses print() for diagnostic output instead of module logger.
What the draft fix changed: Added `import logging` and module-level `logger = logging.getLogger(__name__)` near the top of the file, then replaced all `print(...)` calls in `load_flamingo_guidelines`, `load_custom_instructions`, `load_validation_rules` with `logger.info`/`logger.warning`/`logger.error` as appropriate (e.g. "not set" messages → `logger.info`, missing-file messages → `logger.warning`, exception messages → `logger.error`).
Verify the change is correct and complete; do not refactor unrelated code.

fix confidence: 🟢 90 high — react 👍/👎 to teach the reviewer

try:
path = Path(guidelines_path)
if not path.exists():
print(f"[CodeWiki] Guidelines file not found: {guidelines_path}")
logger.warning(f"Guidelines file not found: {guidelines_path}")
return ""

content = path.read_text(encoding='utf-8')
print(f"[CodeWiki] Loaded Flamingo markdown guidelines ({len(content)} chars)")
logger.info(f"Loaded Flamingo markdown guidelines ({len(content)} chars)")
return content
except Exception as e:
print(f"[CodeWiki] Failed to load guidelines: {e}")
logger.error(f"Failed to load guidelines: {e}")
return ""


Expand Down Expand Up @@ -75,12 +78,12 @@ def sanitize_problematic_patterns(text: str) -> str:
"""
import re

print(f"[DEBUG] sanitize_problematic_patterns called - input length: {len(text)}")
logger.debug(f"sanitize_problematic_patterns called - input length: {len(text)}")

# Count braces before sanitization
open_count_before = text.count('{')
close_count_before = text.count('}')

@flamingo flamingo Bot Aug 24, 2026

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🦩 🟠 Verbose DEBUG-level print() statements left in production sanitization code path

Replaced all print(f"[DEBUG] ...") diagnostic/trace statements in sanitize_problematic_patterns and sanitize_and_escape_format_braces with logger.debug(...) calls (brace counts, preserved-placeholder counts, and content previews), so these no longer print unconditionally to stdout and can be suppressed/leveled via standard logging configuration.

🤖 Prompt for AI agents 
In codewiki/src/be/flamingo_guidelines.py around line 82, review and complete this code-review fix: Verbose DEBUG-level print() statements left in production sanitization code path.
What the draft fix changed: Replaced all `print(f"[DEBUG] ...")` diagnostic/trace statements in `sanitize_problematic_patterns` and `sanitize_and_escape_format_braces` with `logger.debug(...)` calls (brace counts, preserved-placeholder counts, and content previews), so these no longer print unconditionally to stdout and can be suppressed/leveled via standard logging configuration.
Verify the change is correct and complete; do not refactor unrelated code.

fix confidence: 🟡 88 medium — react 👍/👎 to teach the reviewer

print(f"[DEBUG] BEFORE: {{ count={open_count_before}, }} count={close_count_before}")
logger.debug(f" BEFORE: {{ count={open_count_before}, }} count={close_count_before}")

# 1. GitHub Actions syntax: ${{...}} → ${...}
# Use iterative approach for robustness with nested braces
Expand All @@ -104,8 +107,8 @@ def sanitize_problematic_patterns(text: str) -> str:
# Count braces after sanitization
open_count_after = text.count('{')
close_count_after = text.count('}')
print(f"[DEBUG] AFTER: {{ count={open_count_after}, }} count={close_count_after}")
print(f"[DEBUG] Sample (first 200 chars): {text[:200]}")
logger.debug(f" AFTER: {{ count={open_count_after}, }} count={close_count_after}")
logger.debug(f" Sample (first 200 chars): {text[:200]}")

return text

Expand Down Expand Up @@ -146,7 +149,7 @@ def sanitize_and_escape_format_braces(text: str) -> str:
"""
import re

print(f"[DEBUG] sanitize_and_escape_format_braces called - input length: {len(text)}")
logger.debug(f"sanitize_and_escape_format_braces called - input length: {len(text)}")

# STEP 1: SANITIZATION (if not already done)
# This ensures problematic patterns are normalized before we escape braces
Expand All @@ -164,7 +167,7 @@ def preserve_numeric(match):

# Replace all {digit} patterns with markers
text = re.sub(r'\{(\d+)\}', preserve_numeric, text)
print(f"[DEBUG] Preserved {len(numeric_placeholders)} numeric placeholders: {list(numeric_placeholders.values())}")
logger.debug(f" Preserved {len(numeric_placeholders)} numeric placeholders: {list(numeric_placeholders.values())}")

# STEP 3: ESCAPE ALL REMAINING BRACES
# Now escape ALL braces (non-numeric content like {Decision}, {Component})
Expand All @@ -180,8 +183,8 @@ def preserve_numeric(match):
# Count braces after escaping
open_count_after = result.count('{')
close_count_after = result.count('}')
print(f"[DEBUG] AFTER ESCAPING: {{ count={open_count_after}, }} count={close_count_after}")
print(f"[DEBUG] Sample (first 200 chars): {result[:200]}")
logger.debug(f" AFTER ESCAPING: {{ count={open_count_after}, }} count={close_count_after}")
logger.debug(f" Sample (first 200 chars): {result[:200]}")

return result

Expand Down Expand Up @@ -259,16 +262,16 @@ def load_custom_instructions() -> str:
custom_instructions = os.environ.get(CUSTOM_INSTRUCTIONS_ENV_VAR, "")

if not custom_instructions:
print(f"[CodeWiki] {CUSTOM_INSTRUCTIONS_ENV_VAR} not set - continuing without custom instructions")
logger.info(f"{CUSTOM_INSTRUCTIONS_ENV_VAR} not set - continuing without custom instructions")
return ""

print(f"[CodeWiki] Loaded custom repo instructions ({len(custom_instructions)} chars)")
logger.info(f"Loaded custom repo instructions ({len(custom_instructions)} chars)")

# CRITICAL: Sanitize on input - this is the ONLY place text sanitization should happen
# Shell scripts pass raw text, and we handle all sanitization here in Python
# This prevents double-sanitization and ensures consistent behavior
sanitized = sanitize_problematic_patterns(custom_instructions)
print(f"[CodeWiki] Sanitized custom instructions ({len(sanitized)} chars after sanitization)")
logger.info(f"Sanitized custom instructions ({len(sanitized)} chars after sanitization)")

return sanitized

Expand All @@ -291,20 +294,20 @@ def load_validation_rules() -> str:
rules_path = os.environ.get(VALIDATION_RULES_ENV_VAR)

if not rules_path:
print(f"[CodeWiki] {VALIDATION_RULES_ENV_VAR} not set - continuing without validation rules injection")
logger.info(f"{VALIDATION_RULES_ENV_VAR} not set - continuing without validation rules injection")
return ""

try:
path = Path(rules_path)
if not path.exists():
print(f"[CodeWiki] Validation rules file not found: {rules_path}")
logger.warning(f"Validation rules file not found: {rules_path}")
return ""

content = path.read_text(encoding='utf-8')
print(f"[CodeWiki] Loaded markdown validation rules ({len(content)} chars)")
logger.info(f"Loaded markdown validation rules ({len(content)} chars)")
return content
except Exception as e:
print(f"[CodeWiki] Failed to load validation rules: {e}")
logger.error(f"Failed to load validation rules: {e}")
return ""


Expand Down
27 changes: 15 additions & 12 deletions codewiki/src/fe/background_worker.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
import threading
import subprocess
import asyncio
import logging
from datetime import datetime
from pathlib import Path
from queue import Queue
Expand All @@ -23,6 +24,8 @@
from .config import WebAppConfig
from codewiki.src.utils import file_manager

logger = logging.getLogger(__name__)

class BackgroundWorker:
"""Background worker for processing documentation generation jobs."""

Expand All @@ -41,7 +44,7 @@ def start(self):
self.running = True
thread = threading.Thread(target=self._worker_loop, daemon=True)
thread.start()
print("Background worker started")
logger.info("Background worker started")

def stop(self):
"""Stop the background worker."""
Comment on lines 44 to 50

@flamingo flamingo Bot Aug 24, 2026

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🦩 🟠 print() used instead of module logger in background_worker.py

Added import logging and a module-level logger = logging.getLogger(__name__) near the top of codewiki/src/fe/background_worker.py, then replaced every print(...) call across the file (in start, load_job_statuses, _reconstruct_jobs_from_cache, save_job_statuses, _worker_loop, and _process_job) with the equivalent logger.info(...) or logger.error(...) call depending on whether the original message indicated a normal status update or an error/failure condition. No behavior, formatting, or control flow was otherwise changed.

🤖 Prompt for AI agents 
In codewiki/src/fe/background_worker.py around line 39, review and complete this code-review fix: print() used instead of module logger in background_worker.py.
What the draft fix changed: Added `import logging` and a module-level `logger = logging.getLogger(__name__)` near the top of `codewiki/src/fe/background_worker.py`, then replaced every `print(...)` call across the file (in `start`, `load_job_statuses`, `_reconstruct_jobs_from_cache`, `save_job_statuses`, `_worker_loop`, and `_process_job`) with the equivalent `logger.info(...)` or `logger.error(...)` call depending on whether the original message indicated a normal status update or an error/failure condition. No behavior, formatting, or control flow was otherwise changed.
Verify the change is correct and complete; do not refactor unrelated code.

fix confidence: 🟢 90 high — react 👍/👎 to teach the reviewer

Expand Down Expand Up @@ -84,9 +87,9 @@ def load_job_statuses(self):
progress=job_data.get('progress', ''),
docs_path=job_data.get('docs_path')
)
print(f"Loaded {len([j for j in self.job_status.values() if j.status == 'completed'])} completed jobs from disk")
logger.info(f"Loaded {len([j for j in self.job_status.values() if j.status == 'completed'])} completed jobs from disk")
except Exception as e:
print(f"Error loading job statuses: {e}")
logger.error(f"Error loading job statuses: {e}")

def _reconstruct_jobs_from_cache(self):
"""Reconstruct job statuses from cache entries for backward compatibility."""
Expand Down Expand Up @@ -114,14 +117,14 @@ def _reconstruct_jobs_from_cache(self):
)
reconstructed_count += 1
except Exception as e:
print(f"Failed to reconstruct job for {cache_entry.repo_url}: {e}")
logger.error(f"Failed to reconstruct job for {cache_entry.repo_url}: {e}")

if reconstructed_count > 0:
print(f"Reconstructed {reconstructed_count} job statuses from cache")
logger.info(f"Reconstructed {reconstructed_count} job statuses from cache")
self.save_job_statuses()

except Exception as e:
print(f"Error reconstructing jobs from cache: {e}")
logger.error(f"Error reconstructing jobs from cache: {e}")

def save_job_statuses(self):
"""Save job statuses to disk."""
Expand All @@ -145,7 +148,7 @@ def save_job_statuses(self):

file_manager.save_json(data, self.jobs_file)
except Exception as e:
print(f"Error saving job statuses: {e}")
logger.error(f"Error saving job statuses: {e}")

def _worker_loop(self):
"""Main worker loop."""
Expand All @@ -157,7 +160,7 @@ def _worker_loop(self):
else:
time.sleep(1)
except Exception as e:
print(f"Worker error: {e}")
logger.error(f"Worker error: {e}")
time.sleep(1)

def _process_job(self, job_id: str):
Expand Down Expand Up @@ -187,7 +190,7 @@ def _process_job(self, job_id: str):
# Save job status to disk
self.save_job_statuses()

print(f"Job {job_id}: Using cached documentation")
logger.info(f"Job {job_id}: Using cached documentation")
return

# Clone repository
Expand Down Expand Up @@ -236,7 +239,7 @@ def _process_job(self, job_id: str):
# Save job status to disk
self.save_job_statuses()

print(f"Job {job_id}: Documentation generated successfully")
logger.info(f"Job {job_id}: Documentation generated successfully")

except Exception as e:
# Update job status with error
Expand All @@ -245,12 +248,12 @@ def _process_job(self, job_id: str):
job.error_message = str(e)
job.progress = f"Failed: {str(e)}"

print(f"Job {job_id}: Failed with error: {e}")
logger.error(f"Job {job_id}: Failed with error: {e}")

finally:
# Cleanup temporary repository
if 'temp_repo_dir' in locals() and os.path.exists(temp_repo_dir):
try:
subprocess.run(['rm', '-rf', temp_repo_dir], check=True)
except Exception as e:
print(f"Failed to cleanup temp directory: {e}")
logger.error(f"Failed to cleanup temp directory: {e}")