fix(CODEWIKI-005): 4 review findings across 3 files - #40
Conversation
| 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 "" | ||
|
|
There was a problem hiding this comment.
🦩 🟠 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
|
|
||
| # Count braces before sanitization | ||
| open_count_before = text.count('{') | ||
| close_count_before = text.count('}') |
There was a problem hiding this comment.
🦩 🟠 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
|
|
||
| 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: |
There was a problem hiding this comment.
🦩 🟠 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
| 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.""" |
There was a problem hiding this comment.
🦩 🟠 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
Closes 4 review findings across 3 files.
Draft — this is a starting point, not a finished change. The fix required judgment, so read it before trusting it.
codewiki/src/be/flamingo_guidelines.py:39codewiki/src/be/flamingo_guidelines.py:82codewiki/src/be/dependency_analyzer/analysis/analysis_service.py:220codewiki/src/fe/background_worker.py:39What changed — and what was deliberately left — is explained per finding as inline review comments on the lines each finding touched.
Run: https://product-hub.flamingo.so/admin/code-review
Run id:
bef4f5a8-e7f3-478b-8731-2becca2de658Merging this PR is recorded as acceptance of the rule that produced it;
closing it unmerged is recorded as rejection. Both feed rule health, so
closing a wrong suggestion is useful rather than merely tidy.