Ow/refactor context engineering#14
Conversation
There was a problem hiding this comment.
Review by RecurseML
🔍 Review performed on 7c5a214..ad44baa
| Severity | Location | Issue | Delete |
|---|---|---|---|
| src/backend/utils.py:48 | Undefined variable reference error | ||
| src/backend/utils.py:58 | Undefined variable reference error |
✅ Files analyzed, no issues (10)
• .gitignore
• src/backend/agent.py
• src/backend/summarisation/role.py
• src/backend/summarisation/run.py
• src/backend/summariser/README.md
• src/backend/summariser/__init__.py
• src/backend/summariser/pipeline.py
• src/backend/summariser/run.py
• src/backend/summariser/utils.py
• src/models.py
⏭️ Files skipped (2)
| Locations |
|---|
src/backend/__init__.py |
src/backend/summarisation/__init__.py |
| """ | ||
| documents = [] | ||
| if not os.path.exists(directory_path): | ||
| logger.warning(f"Directory {directory_path} does not exist.") |
There was a problem hiding this comment.
NameError: undefined name 'logger' used in function. The function 'read_documents_from_directory' uses the variable 'logger' on lines 48 and 58, but this variable is never defined in the function scope or at module level. While the module defines a 'get_logger()' function that returns a logger instance, it is never called to create the 'logger' variable. When this function is called and the directory doesn't exist (line 48) or when a file read exception occurs (line 58), it will raise: NameError: name 'logger' is not defined. Fix: Add 'logger = get_logger(name)' at the module level (after imports) or at the start of the function.
React with 👍 to tell me that this comment was useful, or 👎 if not (and I'll stop posting more comments like this in the future)
| with open(filepath, 'r', encoding='utf-8') as f: | ||
| documents.append(f.read()) | ||
| except Exception as e: | ||
| logger.warning(f"Skipping file {filename}: {e}") |
There was a problem hiding this comment.
NameError: undefined name 'logger' used in exception handler. The function 'read_documents_from_directory' uses the variable 'logger' on line 58 within an exception handler, but this variable is never defined in the function scope or at module level. While the module defines a 'get_logger()' function that returns a logger instance, it is never called to create the 'logger' variable. When a file read exception occurs, the code will raise: NameError: name 'logger' is not defined. This is the same root cause as the bug on line 48. Fix: Add 'logger = get_logger(name)' at the module level (after imports) or at the start of the function.
React with 👍 to tell me that this comment was useful, or 👎 if not (and I'll stop posting more comments like this in the future)
|
Note: This has not been integrated with the rest of the flow yet, it is part of an app-wide refactor. |
Implements #6 and #7 within the scope of role-only kata tailoring.
Need to raise tickets for employee-focused kata tailoring.
Changes made:
RoleInformation,Stack, andTechnologymodels to lift specific information from contextual documents. RemovesRoleInfo,Team,Roleetc.High-level PR Summary
This PR refactors the context engineering system by introducing new lightweight models (
RoleInformation,Stack, andTechnology) to extract structured information from contextual documents. The changes replace the old class-basedInformationExtractionPipelinewith function-based implementations, reorganize the codebase fromsummarisertosummarisation, improve prompt engineering for role-specific extraction, and add a centralized logging utility. The refactor aims to make the system more development-focused and maintainable.⏱️ Estimated Review Time: 15-30 minutes
💡 Review Order Suggestion
src/models.pysrc/backend/utils.pysrc/backend/__init__.pysrc/backend/summarisation/__init__.pysrc/backend/summarisation/role.pysrc/backend/summarisation/run.pysrc/backend/agent.pysrc/backend/summariser/__init__.pysrc/backend/summariser/pipeline.pysrc/backend/summariser/run.pysrc/backend/summariser/utils.pysrc/backend/summariser/README.md.gitignore.gitignore.DS_STOREandsessions.jsonto gitignore appears to be unrelated cleanup that doesn't directly support the context engineering refactor theme