built- evaluation agent - #4
Conversation
📝 WalkthroughWalkthroughThis PR introduces a complete LangGraph-based evaluation agent system for multi-file compliance assessment. It adds agent orchestration, support for multiple file formats (PDF, CSV, XLSX, PPTX, DOCX), Groq LLM integration, control retrieval tools, PDF report generation, and corresponding API endpoints with response models. Configuration dependencies and documentation are updated accordingly. Changes
Sequence DiagramsequenceDiagram
participant Client
participant API as API Endpoint
participant Service as EvaluationService
participant Agent as LangGraph Agent
participant LLM as Groq LLM
participant Tools as Agent Tools
participant RAG as RAG/Vector DB
participant Report as Report Generator
participant Storage as File Storage
Client->>API: POST /submit<br/>(files, framework_name,<br/>control_ids_per_file)
API->>Service: submit_evaluation()
Service->>Agent: run_evaluation_agent()
Agent->>Storage: Persist input files
Agent->>Agent: Build mimic_json
Agent->>Agent: Initialize EvaluationState
Agent->>Agent: file_processing_node<br/>(extract text)
loop For each file
Agent->>LLM: Build eval prompt<br/>+ invoke with tools
LLM->>Agent: AI response<br/>(tool_calls or done)
alt Has tool calls
Agent->>Tools: Execute tools<br/>(get_control_ids,<br/>retrieve_details)
Tools->>RAG: Query control details
RAG->>Tools: Control metadata
Tools->>Agent: Return ToolMessages
Agent->>LLM: Continue with<br/>tool results
else Done
Agent->>Agent: file_eval_done_node<br/>(parse results)
end
end
Agent->>Report: build_report_pdf()
Report->>Storage: Write PDF
Report->>Agent: report_path
Agent->>Service: Final state<br/>(evaluation_id,<br/>mimic_json, report_path)
Service->>API: Return result
API->>Client: SubmitEvaluationResponse
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes The PR introduces substantial new logic across multiple areas: intricate state management and graph orchestration in graph.py and tools.py, multiple document parsers with similar but distinct implementations, PDF generation with complex formatting, and full API integration. The heterogeneous nature of changes (agent system, parsers, API, documentation, dependencies) combined with dense logic in core modules requires careful review across distinct reasoning contexts. Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 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}
There was a problem hiding this comment.
Actionable comments posted: 12
🤖 Fix all issues with AI agents
In `@services/ai-service/pyproject.toml`:
- Around line 23-31: The dependency list lacks an explicit Pydantic v2
requirement and has a too-permissive langgraph range that can break
compatibility with langchain-core>=0.3.0; update the dependencies in
pyproject.toml by adding "pydantic>=2.0.0" and replace the "langgraph>=0.2.0"
entry with a pinned compatibility range "langgraph>=0.2.20,<0.3.0" so
langchain-core, langgraph, and related packages (groq, langchain-groq,
langchain-core, langgraph) remain compatible.
In `@services/ai-service/src/agent/EVALUATION_AGENT_IMPLEMENTATION.md`:
- Around line 21-28: The markdown table with header "Layer | Path | Role" has
inconsistent separator spacing triggering MD060; fix it by normalizing the
separator row so each column has a matching dash block and consistent
single-space padding around pipes (e.g., change the separator to something like
"|-------|------|------|" so it aligns with the header columns), ensure the
number of columns in the separator matches the header, and keep consistent
spacing in the rows beneath (affecting the table block that lists
Agent/Tools/Parsers/Service/API/App).
In `@services/ai-service/src/agent/Evaluation_Agent_System_Design.md`:
- Around line 9-21: The fenced code block in Evaluation_Agent_System_Design.md
(the flow block starting with "Frontend Submission Page" and ending with
"Generates comprehensive application report") is missing a language identifier;
update that opening triple-backtick to include an appropriate language tag (for
example use ```text for plain flow text or ```mermaid if converting to a
diagram) so markdownlint MD040 is satisfied and rendering improves—locate the
block by the exact contents "Frontend Submission Page ... Generates
comprehensive application report" and add the language tag to the opening fence.
In `@services/ai-service/src/agent/graph.py`:
- Around line 26-33: The loop currently swallows exceptions from
extract_text_from_file and injects the error text into extracted_text; instead,
when extract_text_from_file raises, set extracted_text to None (or empty) and
record a separate extraction_error field (e.g., {"path": path, "extracted_text":
None, "extraction_error": str(e), "field_id": field_id}) in the result list so
downstream logic can detect and skip/flag failed files; update any downstream
consumers to check extraction_error or None extracted_text before sending
content to the LLM.
In `@services/ai-service/src/agent/LANGGRAPH_EVALUATION_FLOW.md`:
- Around line 28-35: The Markdown table containing columns like `evaluation_id`,
`framework_name`, `files`, `mimic_json`, `current_file_index`, and
`file_evaluations` has inconsistent pipe spacing triggering MD060; fix it by
normalizing spacing around every pipe (ensure a single space after and before
each `|`), and make the separator row consistent (e.g., `| --- | --- | --- |`)
so all header and data rows follow the same pipe/space pattern; update the table
in LANGGRAPH_EVALUATION_FLOW.md where those keys appear to match the corrected
spacing.
In `@services/ai-service/src/agent/mimic_json.py`:
- Around line 18-20: The loop building the inner dict silently overwrites
duplicate keys when iterating over field_control_ids; update the logic around
inner and the for field_id, ids_str in field_control_ids loop to guard against
duplicates by checking if field_id already exists in inner and then either raise
a clear exception (e.g., ValueError mentioning the duplicated field_id and the
conflicting ids_str values) or log a warning and decide a deterministic merge
strategy; ensure the message references field_id and the conflicting ids_str so
callers can locate and fix the duplicate input.
In `@services/ai-service/src/agent/report.py`:
- Around line 23-32: evaluation_id is used directly to build path (path =
reports_dir / f"{evaluation_id}.pdf") which risks path traversal or unsafe
filenames; sanitize and normalize evaluation_id before use in
_project_root/reports_dir path creation: validate that evaluation_id contains
only safe characters (e.g., alphanumerics, hyphen, underscore), strip or replace
path separators, and if empty or invalid generate a safe fallback (e.g., a UUID)
and then construct path = reports_dir / f"{safe_evaluation_id}.pdf"; update
references to evaluation_id in this function to use the sanitized
safe_evaluation_id variable.
- Around line 93-101: Escape any user/LLM-derived text before passing into
ReportLab's Paragraph to avoid XML/markup injection: when building the report in
the block that reads summary = ev.get("summary") or ev.get("evaluation") or ""
and the fallback that uses text = str(ev), call xml.sax.saxutils.escape(...) on
the truncated text (before the .replace("\n", "<br/>")) and use that escaped
string in Paragraph(styles["Normal"]) rendering; also add the import for
xml.sax.saxutils.escape at the top of the module.
In `@services/ai-service/src/agent/run.py`:
- Around line 48-53: The loop that writes uploaded files (for i, (filename,
body) in enumerate(files)) currently builds safe_name and writes to eval_dir /
safe_name, which allows absolute paths, path separators, traversal, and
collisions; fix by deriving the base name from the user filename (use the
filename's basename, e.g., Path(filename).name) and reject or strip any path
components or null bytes, then construct the destination via
eval_dir.joinpath(safe_name).resolve() and assert the resolved path startswith
eval_dir.resolve() to prevent traversal; to avoid collisions, if the target path
exists append a short unique suffix (index or uuid) to safe_name until
non-existent; update uses of safe_name, path, file_list entries (path and
field_id generation) accordingly.
In `@services/ai-service/src/api/models/README.md`:
- Line 4: The README currently omits the SubmitEvaluationResponse model from
responses.py; update services/ai-service/src/api/models/README.md to list all
response models including SubmitEvaluationResponse alongside ControlSummary,
SetupFrameworkResponse, ErrorResponse and mention the ExtractionError exception
so the documentation matches the actual symbols defined in responses.py.
In `@services/ai-service/src/api/routers/evaluations.py`:
- Around line 66-70: When iterating over uploads in the loop that builds
file_tuples (the variables files, u, file_tuples and u.filename in this block),
validate that the uploaded file has non-empty content after reading: if u.read()
yields an empty body, raise an HTTPException with status_code=400 and a clear
detail message (e.g., indicate the filename is empty) before appending to
file_tuples; keep the existing check for missing filename and perform the
empty-body validation immediately after reading into body.
In `@services/ai-service/src/processing/tabular_parser.py`:
- Around line 24-49: The code claims to support ".xls" but pandas.read_excel
needs the xlrd engine which isn't in dependencies; remove ".xls" support: update
extract_text_from_tabular to only check for ".csv" and ".xlsx" (drop ".xls" from
the suffix tuple) and update the function/docstring for extract_text_from_xlsx
and extract_text_from_tabular to state only .xlsx is supported, ensuring any
callers/tests expecting ".xls" are adjusted or xlrd is added to dependencies if
you prefer to keep .xls support.
🧹 Nitpick comments (5)
services/ai-service/src/agent/Agent_Quick_Reference.md (1)
14-15: Documentation may diverge from current implementation.The reference document mentions
Qwen2.5-32B (local) + Claude-3.5-Sonnet (API testing)as the LLM stack, but the README.md Environment section indicates the implementation uses Groq withllama-3.3-70b-versatileas the default model. Consider updating this document to reflect the actual implementation or clarifying that this represents future/alternative options.services/ai-service/src/services/__init__.py (1)
6-6: Consider sorting__all__alphabetically.Ruff (RUF022) suggests applying isort-style sorting to
__all__. This is a minor style consistency improvement.🔧 Optional fix
-__all__ = ["FrameworkService", "EvaluationService"] +__all__ = ["EvaluationService", "FrameworkService"]services/ai-service/src/processing/docx_parser.py (1)
18-28: Fail fast when no DOCX text is extracted (consistency with PDF parser). Returning an empty string can silently pass empty content into evaluation. Consider raising a ValueError whenpartsis empty.Proposed tweak
- return "\n\n".join(parts) + if not parts: + raise ValueError(f"No text could be extracted from DOCX: {path}") + return "\n\n".join(parts)services/ai-service/src/processing/pptx_parser.py (1)
18-24: Fail fast when no PPTX text is extracted (consistency with PDF parser). Avoid silently evaluating empty content.Proposed tweak
- return "\n\n".join(p for p in parts if p) + parts = [p for p in parts if p] + if not parts: + raise ValueError(f"No text could be extracted from PPTX: {path}") + return "\n\n".join(parts)services/ai-service/src/agent/tools.py (1)
71-82: Gracefully handle comma-separated control IDs in tool dispatch.LLMs sometimes ignore tool instructions and pass multiple IDs. Routing comma-separated values through the existing
retrieve_control_details_multiimproves robustness without changing the intended flow.🛠️ Proposed fix
if name == "retrieve_control_details": - return rag_retrieve_control_details( - args["control_id"], - args["framework_name"], - top_k_pdf=args.get("top_k_pdf", 5), - ) + control_id = args.get("control_id", "") + framework_name = args.get("framework_name", "") + if "," in control_id: + return retrieve_control_details_multi( + control_id, + framework_name, + top_k_pdf=args.get("top_k_pdf", 5), + ) + return rag_retrieve_control_details( + control_id, + framework_name, + top_k_pdf=args.get("top_k_pdf", 5), + )
| "groq>=0.4.0", | ||
| "langchain-groq>=0.2.0", | ||
| "langgraph>=0.2.0", | ||
| "langchain-core>=0.3.0", | ||
| "reportlab>=4.0.0", | ||
| "python-docx>=1.0.0", | ||
| "python-pptx>=0.6.0", | ||
| "openpyxl>=3.1.0", | ||
| "pandas>=2.0.0", |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check latest versions and verify specified minimums exist on PyPI
packages=(
"groq:0.4.0"
"langchain-groq:0.2.0"
"langgraph:0.2.0"
"langchain-core:0.3.0"
"reportlab:4.0.0"
"python-docx:1.0.0"
"python-pptx:0.6.0"
"openpyxl:3.1.0"
"pandas:2.0.0"
)
for entry in "${packages[@]}"; do
IFS=':' read -r pkg min_version <<< "$entry"
echo "=== $pkg (required: >=$min_version) ==="
response=$(curl -s "https://pypi.org/pypi/$pkg/json" 2>/dev/null)
if [ $? -eq 0 ] && [ -n "$response" ]; then
latest=$(echo "$response" | jq -r '.info.version // "Not found"')
releases=$(echo "$response" | jq -r '.releases | keys[]' 2>/dev/null | grep "^$min_version" | head -1)
echo "Latest version: $latest"
if [ -n "$releases" ]; then
echo "Minimum version $min_version: EXISTS ✓"
else
echo "Checking if minimum version exists..."
all_releases=$(echo "$response" | jq -r '.releases | keys[]' 2>/dev/null | sort -V)
match=$(echo "$all_releases" | grep "^$(echo $min_version | cut -d. -f1-2)" | head -1)
if [ -n "$match" ]; then
echo "Minimum version $min_version: Exists in release history ✓"
else
echo "Minimum version $min_version: May not exist ✗"
fi
fi
else
echo "Error fetching package info"
fi
echo ""
doneRepository: AFAskar/Governance-Agent
Length of output: 909
🌐 Web query:
LangChain ecosystem compatibility langchain-core langchain-groq langgraph 0.3.0 0.2.0 breaking changes
💡 Result:
Python (LangChain / LangChain Core) v0.2 → v0.3
- Only breaking changes in
langchain/langchain-core0.3.x: internal move to Pydantic v2 (Pydantic v1 no longer supported) and Python 3.8 dropped. [1][2]
Python LangGraph v0.2.0 (breaking changes)
LangGraph 0.2.0 introduced multiple breaking changes mostly around checkpointing and stream output:
- Checkpointers split into separate namespace packages (
langgraph-checkpoint-*) and re-export imports removed (import paths changed). [3] thread_ts/parent_tsrenamed tocheckpoint_id/parent_checkpoint_id. [3]- SQLite saver
.from_conn_stringbecame a context manager. [3] - Stream output now includes all nodes, even those that don’t write to state. [3]
Python LangGraph v0.3.0 (breaking changes)
LangGraph 0.3.0 breaking changes:
langgraph.prebuiltwas moved into a separate package:langgraph-prebuilt(you mustpip install langgraph-prebuiltto keep usingfrom langgraph.prebuilt import ...). [4]- Removed
messages_modifierparam forcreate_react_agent(usepromptinstead). [4] - Removed
langgraph.prebuilt.tool_executor(useToolNode). [4]
Compatibility guidance (the practical “what versions work together”)
- If you’re on LangChain /
langchain-core0.3.x, the LangChain migration docs specifically recommend staying on LangGraph>=0.2.20,<0.3(i.e., LangGraph 0.2.x), and note it works with either 0.2 or 0.3 base LangChain packages. [5] - Moving to LangGraph 0.3.x is where you’ll hit the biggest friction (the prebuilt split to
langgraph-prebuilt). [4]
LangGraph.js note (because your query mentions 0.3.0 / 0.2.0)
For LangGraph.js, the project’s “Over Time” page lists v0.2.0 → v0.3.0 breaking changes like interrupt propagation changes and stricter .stream() typing; and separately notes that upgrading to v0.2.0 required explicitly installing @langchain/core. [6]
Sources:
[1] https://blog.langchain.com/announcing-langchain-v0-3/
[2] https://python.langchain.com/docs/versions/v0_3/
[3] https://newreleases.io/project/github/langchain-ai/langgraph/release/0.2.0
[4] https://newreleases.io/project/github/langchain-ai/langgraph/release/0.3.0
[5] https://python.langchain.com/docs/versions/v0_3/
[6] https://langchain-ai.lang.chat/langgraphjs/versions/
Tighten version constraints for LangChain ecosystem compatibility and add Pydantic v2 as explicit dependency.
The specified versions have documented incompatibilities. langchain-core>=0.3.0 requires Pydantic v2 (v1 no longer supported) but Pydantic is not listed as a dependency. Additionally, langgraph>=0.2.0 is too permissive—versions 0.2.0 and 0.3.0 both introduce breaking changes, and the LangChain migration docs recommend staying on langgraph>=0.2.20,<0.3 when using langchain-core>=0.3.0 to avoid the langgraph-prebuilt split and other breaking changes in 0.3.0.
Recommended changes:
- Add
"pydantic>=2.0.0"as explicit dependency - Change
"langgraph>=0.2.0"to"langgraph>=0.2.20,<0.3.0"to prevent compatibility issues
🤖 Prompt for AI Agents
In `@services/ai-service/pyproject.toml` around lines 23 - 31, The dependency list
lacks an explicit Pydantic v2 requirement and has a too-permissive langgraph
range that can break compatibility with langchain-core>=0.3.0; update the
dependencies in pyproject.toml by adding "pydantic>=2.0.0" and replace the
"langgraph>=0.2.0" entry with a pinned compatibility range
"langgraph>=0.2.20,<0.3.0" so langchain-core, langgraph, and related packages
(groq, langchain-groq, langchain-core, langgraph) remain compatible.
| | Layer | Path | Role | | ||
| |-------|------|------| | ||
| | Agent | `src/agent/*.py` | LangGraph graph, state, tools, groq_client, mimic_json, report, run | | ||
| | Tools | `src/agent/tools.py` | (1) get_control_ids_for_file (from mimic JSON), (2) retrieve_control_details (vector DB via `src.rag`) | | ||
| | Parsers | `src/processing/` | pdf_parser (existing), tabular_parser, pptx_parser, docx_parser, file_dispatcher | | ||
| | Service | `src/services/evaluation_service.py` | submit_evaluation → run_evaluation_agent | | ||
| | API | `src/api/routers/evaluations.py` | POST /api/v1/evaluations/submit | | ||
| | App | `src/api/app.py` | Registers evaluations router; startup creates `data/evaluations` and `data/reports` | |
There was a problem hiding this comment.
Fix markdown table separator spacing to satisfy MD060.
📝 Suggested fix
-|-------|------|------|
+| ------ | ------ | ------ |📝 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.
| | Layer | Path | Role | | |
| |-------|------|------| | |
| | Agent | `src/agent/*.py` | LangGraph graph, state, tools, groq_client, mimic_json, report, run | | |
| | Tools | `src/agent/tools.py` | (1) get_control_ids_for_file (from mimic JSON), (2) retrieve_control_details (vector DB via `src.rag`) | | |
| | Parsers | `src/processing/` | pdf_parser (existing), tabular_parser, pptx_parser, docx_parser, file_dispatcher | | |
| | Service | `src/services/evaluation_service.py` | submit_evaluation → run_evaluation_agent | | |
| | API | `src/api/routers/evaluations.py` | POST /api/v1/evaluations/submit | | |
| | App | `src/api/app.py` | Registers evaluations router; startup creates `data/evaluations` and `data/reports` | | |
| | Layer | Path | Role | | |
| | ------ | ------ | ------ | | |
| | Agent | `src/agent/*.py` | LangGraph graph, state, tools, groq_client, mimic_json, report, run | | |
| | Tools | `src/agent/tools.py` | (1) get_control_ids_for_file (from mimic JSON), (2) retrieve_control_details (vector DB via `src.rag`) | | |
| | Parsers | `src/processing/` | pdf_parser (existing), tabular_parser, pptx_parser, docx_parser, file_dispatcher | | |
| | Service | `src/services/evaluation_service.py` | submit_evaluation → run_evaluation_agent | | |
| | API | `src/api/routers/evaluations.py` | POST /api/v1/evaluations/submit | | |
| | App | `src/api/app.py` | Registers evaluations router; startup creates `data/evaluations` and `data/reports` | |
🧰 Tools
🪛 markdownlint-cli2 (0.20.0)
[warning] 22-22: Table column style
Table pipe is missing space to the right for style "compact"
(MD060, table-column-style)
[warning] 22-22: Table column style
Table pipe is missing space to the left for style "compact"
(MD060, table-column-style)
[warning] 22-22: Table column style
Table pipe is missing space to the right for style "compact"
(MD060, table-column-style)
[warning] 22-22: Table column style
Table pipe is missing space to the left for style "compact"
(MD060, table-column-style)
[warning] 22-22: Table column style
Table pipe is missing space to the right for style "compact"
(MD060, table-column-style)
[warning] 22-22: Table column style
Table pipe is missing space to the left for style "compact"
(MD060, table-column-style)
🤖 Prompt for AI Agents
In `@services/ai-service/src/agent/EVALUATION_AGENT_IMPLEMENTATION.md` around
lines 21 - 28, The markdown table with header "Layer | Path | Role" has
inconsistent separator spacing triggering MD060; fix it by normalizing the
separator row so each column has a matching dash block and consistent
single-space padding around pipes (e.g., change the separator to something like
"|-------|------|------|" so it aligns with the header columns), ensure the
number of columns in the separator matches the header, and keep consistent
spacing in the rows beneath (affecting the table block that lists
Agent/Tools/Parsers/Service/API/App).
| ``` | ||
| Frontend Submission Page | ||
| ↓ | ||
| 15 File Upload Fields (CSV, PPTX, DOCX, PDF, XLSX) | ||
| ↓ | ||
| Backend receives: {file1: [control_ids], file2: [control_ids], ...} | ||
| ↓ | ||
| Evaluation Agent (RAG + Tools) | ||
| ↓ | ||
| Evaluates each file against assigned controls | ||
| ↓ | ||
| Generates comprehensive application report | ||
| ``` |
There was a problem hiding this comment.
Add language identifiers to fenced code blocks (MD040).
markdownlint flagged multiple code fences without a language. Apply a language tag (e.g., text, mermaid, json) across the doc to satisfy linting and improve rendering.
✅ Example fix
-```
+```text
Frontend Submission Page
↓
15 File Upload Fields (CSV, PPTX, DOCX, PDF, XLSX)
↓
Backend receives: {file1: [control_ids], file2: [control_ids], ...}
↓
Evaluation Agent (RAG + Tools)
↓
Evaluates each file against assigned controls
↓
Generates comprehensive application report
-```
+```🧰 Tools
🪛 markdownlint-cli2 (0.20.0)
[warning] 9-9: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
🤖 Prompt for AI Agents
In `@services/ai-service/src/agent/Evaluation_Agent_System_Design.md` around lines
9 - 21, The fenced code block in Evaluation_Agent_System_Design.md (the flow
block starting with "Frontend Submission Page" and ending with "Generates
comprehensive application report") is missing a language identifier; update that
opening triple-backtick to include an appropriate language tag (for example use
```text for plain flow text or ```mermaid if converting to a diagram) so
markdownlint MD040 is satisfied and rendering improves—locate the block by the
exact contents "Frontend Submission Page ... Generates comprehensive application
report" and add the language tag to the opening fence.
| for i, f in enumerate(files): | ||
| path = f.get("path") or "" | ||
| field_id = f.get("field_id") or f"field_{i + 1}" | ||
| try: | ||
| text = extract_text_from_file(path) | ||
| except Exception as e: | ||
| text = f"[Extraction error: {e}]" | ||
| result.append({"path": path, "extracted_text": text, "field_id": field_id}) |
There was a problem hiding this comment.
Capture extraction failures explicitly instead of feeding error text to the LLM.
Lines 29-33 convert exceptions into content and proceed, which can produce misleading compliance results while hiding errors. Prefer recording the error in state and storing it per file so downstream steps can skip or flag failed extractions.
🧯 Proposed fix
- result = []
+ result = []
+ errors = list(state.get("errors") or [])
for i, f in enumerate(files):
path = f.get("path") or ""
field_id = f.get("field_id") or f"field_{i + 1}"
try:
text = extract_text_from_file(path)
- except Exception as e:
- text = f"[Extraction error: {e}]"
- result.append({"path": path, "extracted_text": text, "field_id": field_id})
+ error = None
+ except Exception as e:
+ error = f"Extraction error for {path}: {e}"
+ errors.append(error)
+ text = ""
+ result.append({"path": path, "extracted_text": text, "field_id": field_id, "error": error})
return {
"files": result,
"current_file_index": 0,
"file_evaluations": [],
+ "errors": errors,
}🧰 Tools
🪛 Ruff (0.14.14)
[warning] 31-31: Do not catch blind exception: Exception
(BLE001)
🤖 Prompt for AI Agents
In `@services/ai-service/src/agent/graph.py` around lines 26 - 33, The loop
currently swallows exceptions from extract_text_from_file and injects the error
text into extracted_text; instead, when extract_text_from_file raises, set
extracted_text to None (or empty) and record a separate extraction_error field
(e.g., {"path": path, "extracted_text": None, "extraction_error": str(e),
"field_id": field_id}) in the result list so downstream logic can detect and
skip/flag failed files; update any downstream consumers to check
extraction_error or None extracted_text before sending content to the LLM.
| | Key | Type | Description | | ||
| |-----|------|-------------| | ||
| | `evaluation_id` | str | UUID for this evaluation run | | ||
| | `framework_name` | str | Framework identifier (e.g. NDI) | | ||
| | `files` | list[dict] | `[{path, extracted_text, field_id}, ...]` per file | | ||
| | `mimic_json` | dict | `{framework_name: {field_1: "id1,id2", field_2: "..."}}` | | ||
| | `current_file_index` | int | Index of file being evaluated (0-based) | | ||
| | `file_evaluations` | list[dict] | Accumulated per-file results | |
There was a problem hiding this comment.
Fix table pipe spacing to satisfy markdownlint (MD060).
The table formatting around Line 28 triggers MD060. Apply consistent spacing across tables to keep linting clean.
✍️ Example fix
-| Key | Type | Description |
-|-----|------|-------------|
+| Key | Type | Description |
+| --- | ---- | ----------- |📝 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.
| | Key | Type | Description | | |
| |-----|------|-------------| | |
| | `evaluation_id` | str | UUID for this evaluation run | | |
| | `framework_name` | str | Framework identifier (e.g. NDI) | | |
| | `files` | list[dict] | `[{path, extracted_text, field_id}, ...]` per file | | |
| | `mimic_json` | dict | `{framework_name: {field_1: "id1,id2", field_2: "..."}}` | | |
| | `current_file_index` | int | Index of file being evaluated (0-based) | | |
| | `file_evaluations` | list[dict] | Accumulated per-file results | | |
| | Key | Type | Description | | |
| | --- | ---- | ----------- | | |
| | `evaluation_id` | str | UUID for this evaluation run | | |
| | `framework_name` | str | Framework identifier (e.g. NDI) | | |
| | `files` | list[dict] | `[{path, extracted_text, field_id}, ...]` per file | | |
| | `mimic_json` | dict | `{framework_name: {field_1: "id1,id2", field_2: "..."}}` | | |
| | `current_file_index` | int | Index of file being evaluated (0-based) | | |
| | `file_evaluations` | list[dict] | Accumulated per-file results | |
🧰 Tools
🪛 markdownlint-cli2 (0.20.0)
[warning] 29-29: Table column style
Table pipe is missing space to the right for style "compact"
(MD060, table-column-style)
[warning] 29-29: Table column style
Table pipe is missing space to the left for style "compact"
(MD060, table-column-style)
[warning] 29-29: Table column style
Table pipe is missing space to the right for style "compact"
(MD060, table-column-style)
[warning] 29-29: Table column style
Table pipe is missing space to the left for style "compact"
(MD060, table-column-style)
[warning] 29-29: Table column style
Table pipe is missing space to the right for style "compact"
(MD060, table-column-style)
[warning] 29-29: Table column style
Table pipe is missing space to the left for style "compact"
(MD060, table-column-style)
🤖 Prompt for AI Agents
In `@services/ai-service/src/agent/LANGGRAPH_EVALUATION_FLOW.md` around lines 28 -
35, The Markdown table containing columns like `evaluation_id`,
`framework_name`, `files`, `mimic_json`, `current_file_index`, and
`file_evaluations` has inconsistent pipe spacing triggering MD060; fix it by
normalizing spacing around every pipe (ensure a single space after and before
each `|`), and make the separator row consistent (e.g., `| --- | --- | --- |`)
so all header and data rows follow the same pipe/space pattern; update the table
in LANGGRAPH_EVALUATION_FLOW.md where those keys appear to match the corrected
spacing.
| summary = ev.get("summary") or ev.get("evaluation") or "" | ||
| if summary: | ||
| text = summary if len(summary) <= 2000 else summary[:2000] + "..." | ||
| story.append(Paragraph(text.replace("\n", "<br/>"), styles["Normal"])) | ||
| elif not control_decisions: | ||
| text = str(ev) | ||
| if len(text) > 2000: | ||
| text = text[:2000] + "..." | ||
| story.append(Paragraph(text.replace("\n", "<br/>"), styles["Normal"])) |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, let's check the file exists and examine the code context
cat -n services/ai-service/src/agent/report.py | sed -n '85,110p'Repository: AFAskar/Governance-Agent
Length of output: 1137
🏁 Script executed:
# Let's also check the imports in the file to understand what's being used
head -30 services/ai-service/src/agent/report.pyRepository: AFAskar/Governance-Agent
Length of output: 1083
🌐 Web query:
ReportLab Paragraph markup XML special characters escaping
💡 Result:
ReportLab platypus.Paragraph text is parsed as XML-like markup, so you must escape XML special characters in any literal text (unless they’re part of actual <b>...</b> / <font ...> etc. tags). (docs.reportlab.com)
Escape rules (use these entities)
&→&<→<>→>"→"(mainly inside attribute values)'→'(mainly inside attribute values) (stackoverflow.com)
Example:
from reportlab.platypus import Paragraph
from reportlab.lib.styles import getSampleStyleSheet
styles = getSampleStyleSheet()
p = Paragraph("AT&T < Verizon", styles["Normal"])Convenient escaping in Python
Use xml.sax.saxutils.escape (good for text nodes; add quotes mapping if needed):
from xml.sax.saxutils import escape
text = '5 < 6 & 7 > 3'
safe = escape(text) # '5 < 6 & 7 > 3'If you’re embedding markup tags, escape only the user/content parts, not the tags (otherwise you’ll neutralize your <b>, <i>, etc.). (docs.reportlab.com)
Citations:
- 1: https://docs.reportlab.com/reportlab/userguide/ch6_paragraphs/?utm_source=openai
- 2: https://stackoverflow.com/questions/74443398/prevent-escaping-characters-in-an-xml-string?utm_source=openai
- 3: https://docs.reportlab.com/reportlab/userguide/ch6_paragraphs/?utm_source=openai
Escape summary and evaluation text before Paragraph rendering.
ReportLab's Paragraph interprets text as XML-like markup, so unescaped &, <, and > characters from LLM or file-derived text will break PDF generation or distort output. Use xml.sax.saxutils.escape() before passing to Paragraph:
Proposed fix
from reportlab.lib import colors
from reportlab.lib.pagesizes import A4
from reportlab.lib.styles import getSampleStyleSheet
+from xml.sax.saxutils import escape
from reportlab.platypus import SimpleDocTemplate, Paragraph, Spacer, Table, TableStyle
@@
story = []
+
+ def _escape_para(text: str) -> str:
+ return escape(str(text)).replace("\n", "<br/>")
@@
text = summary if len(summary) <= 2000 else summary[:2000] + "..."
- story.append(Paragraph(text.replace("\n", "<br/>"), styles["Normal"]))
+ story.append(Paragraph(_escape_para(text), styles["Normal"]))
@@
text = str(ev)
if len(text) > 2000:
text = text[:2000] + "..."
- story.append(Paragraph(text.replace("\n", "<br/>"), styles["Normal"]))
+ story.append(Paragraph(_escape_para(text), styles["Normal"]))🤖 Prompt for AI Agents
In `@services/ai-service/src/agent/report.py` around lines 93 - 101, Escape any
user/LLM-derived text before passing into ReportLab's Paragraph to avoid
XML/markup injection: when building the report in the block that reads summary =
ev.get("summary") or ev.get("evaluation") or "" and the fallback that uses text
= str(ev), call xml.sax.saxutils.escape(...) on the truncated text (before the
.replace("\n", "<br/>")) and use that escaped string in
Paragraph(styles["Normal"]) rendering; also add the import for
xml.sax.saxutils.escape at the top of the module.
| for i, (filename, body) in enumerate(files): | ||
| safe_name = (filename or f"file_{i+1}").replace("..", "_").strip() or f"file_{i+1}" | ||
| path = eval_dir / safe_name | ||
| path.write_bytes(body) | ||
| field_id = f"field_{i + 1}" | ||
| file_list.append({"path": str(path), "extracted_text": "", "field_id": field_id}) |
There was a problem hiding this comment.
Harden filename handling to prevent path traversal and collisions.
User-controlled filenames can be absolute or include separators; eval_dir / safe_name can escape the evaluation directory. Also, duplicate filenames overwrite earlier uploads.
🔒 Suggested fix
- for i, (filename, body) in enumerate(files):
- safe_name = (filename or f"file_{i+1}").replace("..", "_").strip() or f"file_{i+1}"
- path = eval_dir / safe_name
+ eval_root = eval_dir.resolve()
+ for i, (filename, body) in enumerate(files):
+ base_name = Path(filename or f"file_{i+1}").name
+ base_name = base_name.replace("..", "_").replace("/", "_").replace("\\", "_").strip() or f"file_{i+1}"
+ safe_name = f"{i+1}_{base_name}"
+ path = (eval_dir / safe_name).resolve()
+ if eval_root not in path.parents:
+ raise ValueError("Invalid filename")
path.write_bytes(body)🤖 Prompt for AI Agents
In `@services/ai-service/src/agent/run.py` around lines 48 - 53, The loop that
writes uploaded files (for i, (filename, body) in enumerate(files)) currently
builds safe_name and writes to eval_dir / safe_name, which allows absolute
paths, path separators, traversal, and collisions; fix by deriving the base name
from the user filename (use the filename's basename, e.g., Path(filename).name)
and reject or strip any path components or null bytes, then construct the
destination via eval_dir.joinpath(safe_name).resolve() and assert the resolved
path startswith eval_dir.resolve() to prevent traversal; to avoid collisions, if
the target path exists append a short unique suffix (index or uuid) to safe_name
until non-existent; update uses of safe_name, path, file_list entries (path and
field_id generation) accordingly.
| # api/models — Request/Response Schemas | ||
|
|
||
| - **requests.py** — Placeholder; form/file params are validated in routers (Form, File). | ||
| - **responses.py** — ControlSummary, SetupFrameworkResponse, ErrorResponse; ExtractionError exception for extraction failures. |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
rg -n "^class \w+" services/ai-service/src/api/models/responses.pyRepository: AFAskar/Governance-Agent
Length of output: 264
Update README to include all response models.
responses.py defines SubmitEvaluationResponse for the evaluation endpoint, but it's not listed in the README. Update the documentation to be complete.
📝 Suggested update
-- **responses.py** — ControlSummary, SetupFrameworkResponse, ErrorResponse; ExtractionError exception for extraction failures.
+- **responses.py** — ControlSummary, SetupFrameworkResponse, SubmitEvaluationResponse, ErrorResponse; ExtractionError exception for extraction failures.📝 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.
| - **responses.py** — ControlSummary, SetupFrameworkResponse, ErrorResponse; ExtractionError exception for extraction failures. | |
| - **responses.py** — ControlSummary, SetupFrameworkResponse, SubmitEvaluationResponse, ErrorResponse; ExtractionError exception for extraction failures. |
🤖 Prompt for AI Agents
In `@services/ai-service/src/api/models/README.md` at line 4, The README currently
omits the SubmitEvaluationResponse model from responses.py; update
services/ai-service/src/api/models/README.md to list all response models
including SubmitEvaluationResponse alongside ControlSummary,
SetupFrameworkResponse, ErrorResponse and mention the ExtractionError exception
so the documentation matches the actual symbols defined in responses.py.
| for u in files: | ||
| if not u.filename: | ||
| raise HTTPException(status_code=400, detail="Each file must have a filename") | ||
| body = await u.read() | ||
| file_tuples.append((u.filename, body)) |
There was a problem hiding this comment.
Reject empty uploads for clearer client errors.
✅ Suggested fix
for u in files:
if not u.filename:
raise HTTPException(status_code=400, detail="Each file must have a filename")
body = await u.read()
+ if not body:
+ raise HTTPException(status_code=400, detail=f"File '{u.filename}' is empty")
file_tuples.append((u.filename, body))🤖 Prompt for AI Agents
In `@services/ai-service/src/api/routers/evaluations.py` around lines 66 - 70,
When iterating over uploads in the loop that builds file_tuples (the variables
files, u, file_tuples and u.filename in this block), validate that the uploaded
file has non-empty content after reading: if u.read() yields an empty body,
raise an HTTPException with status_code=400 and a clear detail message (e.g.,
indicate the filename is empty) before appending to file_tuples; keep the
existing check for missing filename and perform the empty-body validation
immediately after reading into body.
| def extract_text_from_xlsx(path: str) -> str: | ||
| """ | ||
| Extract text from an XLSX file. Reads first sheet; returns table as plain text. | ||
| """ | ||
| import pandas as pd | ||
|
|
||
| p = Path(path) | ||
| if not p.exists(): | ||
| raise FileNotFoundError(f"XLSX file not found: {path}") | ||
| try: | ||
| df = pd.read_excel(path, sheet_name=0) | ||
| return df.to_string(index=False) | ||
| except Exception as e: | ||
| raise ValueError(f"Could not read XLSX {path}: {e}") from e | ||
|
|
||
|
|
||
| def extract_text_from_tabular(path: str) -> str: | ||
| """ | ||
| Dispatch by extension: .csv -> extract_text_from_csv, .xlsx/.xls -> extract_text_from_xlsx. | ||
| """ | ||
| suffix = Path(path).suffix.lower() | ||
| if suffix == ".csv": | ||
| return extract_text_from_csv(path) | ||
| if suffix in (".xlsx", ".xls"): | ||
| return extract_text_from_xlsx(path) | ||
| raise ValueError(f"Unsupported tabular extension: {suffix}") |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Find dependency files in the repository
find . -maxdepth 2 -type f \( -name "requirements*.txt" -o -name "setup.py" -o -name "setup.cfg" -o -name "pyproject.toml" -o -name "Pipfile" -o -name "poetry.lock" -o -name "tox.ini" \) 2>/dev/null | head -20Repository: AFAskar/Governance-Agent
Length of output: 50
🏁 Script executed:
#!/bin/bash
# Search for xlrd and openpyxl references across the codebase
rg -n "openpyxl|xlrd" -S --type pyRepository: AFAskar/Governance-Agent
Length of output: 50
🏁 Script executed:
#!/bin/bash
# Search for all files that might contain dependencies
find . -type f \( -name "*.txt" -o -name "*.toml" -o -name "*.cfg" -o -name "*.ini" -o -name "*.yaml" -o -name "*.yml" \) 2>/dev/null | grep -E "(requirement|depend|setup|pyproject|poetry|lock)" | head -30Repository: AFAskar/Governance-Agent
Length of output: 121
🏁 Script executed:
#!/bin/bash
# Check the pyproject.toml file to see declared dependencies
cat -n ./services/ai-service/pyproject.tomlRepository: AFAskar/Governance-Agent
Length of output: 1162
🏁 Script executed:
#!/bin/bash
# Verify the actual function implementation to confirm the issue
cat -n ./services/ai-service/src/processing/tabular_parser.py | head -50Repository: AFAskar/Governance-Agent
Length of output: 1829
.xls support will fail at runtime—xlrd is not in project dependencies.
The function advertises .xls support in both its docstring and logic (line 47), but pandas.read_excel() requires xlrd to handle .xls files. Only openpyxl (for .xlsx) is declared in pyproject.toml. Remove .xls from the supported extensions or add xlrd to dependencies.
Suggested fix: remove .xls support
def extract_text_from_tabular(path: str) -> str:
"""
- Dispatch by extension: .csv -> extract_text_from_csv, .xlsx/.xls -> extract_text_from_xlsx.
+ Dispatch by extension: .csv -> extract_text_from_csv, .xlsx -> extract_text_from_xlsx.
"""
suffix = Path(path).suffix.lower()
if suffix == ".csv":
return extract_text_from_csv(path)
- if suffix in (".xlsx", ".xls"):
+ if suffix == ".xlsx":
return extract_text_from_xlsx(path)
raise ValueError(f"Unsupported tabular extension: {suffix}")📝 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.
| def extract_text_from_xlsx(path: str) -> str: | |
| """ | |
| Extract text from an XLSX file. Reads first sheet; returns table as plain text. | |
| """ | |
| import pandas as pd | |
| p = Path(path) | |
| if not p.exists(): | |
| raise FileNotFoundError(f"XLSX file not found: {path}") | |
| try: | |
| df = pd.read_excel(path, sheet_name=0) | |
| return df.to_string(index=False) | |
| except Exception as e: | |
| raise ValueError(f"Could not read XLSX {path}: {e}") from e | |
| def extract_text_from_tabular(path: str) -> str: | |
| """ | |
| Dispatch by extension: .csv -> extract_text_from_csv, .xlsx/.xls -> extract_text_from_xlsx. | |
| """ | |
| suffix = Path(path).suffix.lower() | |
| if suffix == ".csv": | |
| return extract_text_from_csv(path) | |
| if suffix in (".xlsx", ".xls"): | |
| return extract_text_from_xlsx(path) | |
| raise ValueError(f"Unsupported tabular extension: {suffix}") | |
| def extract_text_from_tabular(path: str) -> str: | |
| """ | |
| Dispatch by extension: .csv -> extract_text_from_csv, .xlsx -> extract_text_from_xlsx. | |
| """ | |
| suffix = Path(path).suffix.lower() | |
| if suffix == ".csv": | |
| return extract_text_from_csv(path) | |
| if suffix == ".xlsx": | |
| return extract_text_from_xlsx(path) | |
| raise ValueError(f"Unsupported tabular extension: {suffix}") |
🧰 Tools
🪛 Ruff (0.14.14)
[warning] 32-32: Avoid specifying long messages outside the exception class
(TRY003)
[warning] 37-37: Avoid specifying long messages outside the exception class
(TRY003)
[warning] 49-49: Avoid specifying long messages outside the exception class
(TRY003)
🤖 Prompt for AI Agents
In `@services/ai-service/src/processing/tabular_parser.py` around lines 24 - 49,
The code claims to support ".xls" but pandas.read_excel needs the xlrd engine
which isn't in dependencies; remove ".xls" support: update
extract_text_from_tabular to only check for ".csv" and ".xlsx" (drop ".xls" from
the suffix tuple) and update the function/docstring for extract_text_from_xlsx
and extract_text_from_tabular to state only .xlsx is supported, ensuring any
callers/tests expecting ".xls" are adjusted or xlrd is added to dependencies if
you prefer to keep .xls support.
1 participant
finished first functional version of evaluation agent
Summary by CodeRabbit
Release Notes
New Features
Documentation
Chores
✏️ Tip: You can customize this high-level summary in your review settings.