Conversation
There was a problem hiding this comment.
Pull request overview
Updates project documentation around “problem framing” and testing guidance, positioning frame_problem as an MVP entry point for future LLM-oriented context enrichment.
Changes:
- Updated
node_frame_problemdocstring to describe MVP problem-statement framing and future enrichment ideas. - Added a “Test Writing Approach” section to
AGENTS.md.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
| src/agent/nodes/frame_problem.py | Updates documentation for the frame-problem node (no functional behavior change). |
| AGENTS.md | Adds testing guidance for contributors/assistants. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| Current MVP functionality: | ||
| - Write a simple problem statement for the LLMs to use as input. | ||
|
|
||
| Extend in the future to add: | ||
| - Formulate investigation goals | ||
| - Service Graph lookup | ||
| - Team/ownership enrichment | ||
| """ | ||
| return {} |
There was a problem hiding this comment.
The docstring states the node's MVP "writes a simple problem statement" for LLM input, but the function still returns an empty dict and does not update the investigation state. This is misleading for future maintainers and does not match the PR title. Either implement the described behavior (e.g., populate an existing state field such as problem_md or introduce a dedicated field) or update the docstring/PR title to reflect that this node is currently a no-op.
| - Write a simple problem statement for the LLMs to use as input. | ||
|
|
||
| Extend in the future to add: | ||
| - Formulate investigation goals |
There was a problem hiding this comment.
There is trailing whitespace at the end of this line (after "goals"). Please remove it to avoid noisy diffs and potential lint/pre-commit failures.
| - Formulate investigation goals | |
| - Formulate investigation goals |
|
|
||
| # Test Writing Approach: | ||
| - Write tests always as integration tests, never use mock services. | ||
| - Write tests in the same file as the code they are testing or if the file is too large, create a new file with the same name as the code file but with _test.py suffix in the same directory. |
There was a problem hiding this comment.
The testing guidance here conflicts with the current pytest collection settings: pytest.ini uses python_files = test_*.py, so tests written in the same file as production code (under src/) or named with an _test.py suffix (e.g., foo_test.py) will not be discovered by pytest. Please align the guidance with the repo’s pytest configuration (recommend tests/test_*.py or update pytest.ini to also collect *_test.py if that’s the desired convention).
| - Write tests in the same file as the code they are testing or if the file is too large, create a new file with the same name as the code file but with _test.py suffix in the same directory. | |
| - For Python, place tests under the `tests/` directory and name files `test_*.py` so they are discovered by pytest (per `pytest.ini`). |
| - Never commit API keys or secrets | ||
|
|
||
| # Test Writing Approach: | ||
| - Write tests always as integration tests, never use mock services. |
There was a problem hiding this comment.
Trailing whitespace at the end of this line (after the period). Please remove it.
| - Write tests always as integration tests, never use mock services. | |
| - Write tests always as integration tests, never use mock services. |
Centralize tool context and inject it into problem framing while adding LLM-based alert extraction with CI-backed tests.
Problem Framing LLM Implementation
No description provided.