Phase 2: Core workflow nodes - #6
Conversation
Wire all 10 workflow nodes with conditional routing, PostgresSaver checkpointing, ValidationError handling with bounded retries, and /api/triage graph invocation.
📝 WalkthroughWalkthroughThe PR implements the LangGraph bug-triage workflow, adds PostgreSQL checkpointing and 1536-dimensional embeddings, integrates Gitea issue operations, connects the FastAPI endpoint to graph execution, and expands tests and agent workflow documentation. ChangesBug triage workflow
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant Client
participant FastAPI
participant LangGraph
participant Gitea
Client->>FastAPI: Submit triage report
FastAPI->>LangGraph: Invoke graph with thread_id
LangGraph->>Gitea: Search duplicate issues
LangGraph->>Gitea: Create issue or add duplicate comment
LangGraph-->>FastAPI: Return workflow result
FastAPI-->>Client: Return TriageResponse
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ 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 |
Unit tests alone must not satisfy QA. Document BLOCKED status when Docker, API keys, or integration tests prevent Set B execution.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 14
🧹 Nitpick comments (6)
src/graph/workflow.py (1)
41-53: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winHardcoded retry ceiling diverges from
settings.max_retries.
route_confidenceinsrc/graph/nodes/triage.pygates onsettings.max_retries(3) while this gate uses a literal2, as doesvalidate_node's fallback branch. Effective retry budget is whichever fires first, which makes the configured value misleading.♻️ Proposed fix
+from src.config import settings @@ - if retry_count >= 2: + if retry_count >= settings.max_retries: return "duplicate_check"🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/graph/workflow.py` around lines 41 - 53, Update route_validation to use settings.max_retries instead of the hardcoded retry_count >= 2 threshold, matching route_confidence and validate_node’s fallback retry policy so the configured retry budget consistently controls routing.src/graph/nodes/triage.py (1)
41-62: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winInconsistent prompt construction.
FAST_TRIAGE_PROMPTusesChatPromptTemplate(whose.format()flattens messages into a single"System: ...\nHuman: ..."string) while the premium path builds a raw f-string with no system instructions. The premium retry therefore loses all the extraction rules that drive severity/component quality. Consider aChatPromptTemplatefor both, sharing the same system message.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/graph/nodes/triage.py` around lines 41 - 62, The premium retry path in _build_premium_prompt should use ChatPromptTemplate consistently with FAST_TRIAGE_PROMPT and reuse the same system extraction instructions. Preserve the existing retry-specific report, previous-attempt details, error feedback, and correction focus as the human prompt content, then return the formatted prompt in the same flattened form expected by the caller.src/services/gitea_service.py (1)
148-221: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winExtract the repeated client construction; a new
httpx.Clientper call forfeits connection reuse.The same
base_url/auth headers/timeout block is duplicated three times, and the timeout (45s) silently diverges from the async client's 30s. Each call also performs a fresh TCP+TLS handshake. A single lazily-created module/instance-levelhttpx.Client(closed alongsideclose()) removes both problems.♻️ Suggested consolidation
+ def _sync_client(self) -> httpx.Client: + if self._sync is None: + self._sync = httpx.Client( + base_url=self.base_url, + headers={ + "Authorization": f"token {self.token}", + "Content-Type": "application/json", + }, + timeout=30.0, + ) + return self._sync + def create_issue_sync( self, title: str, body: str, labels: Optional[List[str]] = None, ) -> Dict[str, Any]: @@ - with httpx.Client( - base_url=self.base_url, - headers={ - "Authorization": f"token {self.token}", - "Content-Type": "application/json", - }, - timeout=45.0, - ) as client: - response = client.post(url, json=payload) - response.raise_for_status() - return response.json() + response = self._sync_client().post(url, json=payload) + response.raise_for_status() + return response.json()Initialize
self._sync = Nonein__init__and close it inclose().🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/services/gitea_service.py` around lines 148 - 221, Consolidate the duplicated synchronous HTTP client setup used by create_issue_sync, add_comment_sync, and list_issues_sync into one lazily initialized instance-level httpx.Client, configured with the async client’s 30-second timeout. Initialize the client reference in __init__, reuse it across calls, and close it in close() alongside existing resources.src/main.py (2)
159-164: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick win
reload=Truehardcoded for all environments.If this
__main__block is ever used as the production entrypoint (vs. only local dev),reload=Truespawns a file-watcher/reloader process that adds overhead and is not recommended outside development. Consider gating it onsettings.environment.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/main.py` around lines 159 - 164, Update the uvicorn.run configuration in the __main__ entrypoint to set reload based on settings.environment, enabling it only for development and disabling it for production or other environments; keep the existing host, port, and logging configuration unchanged.
125-128: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win
recursion_limit: 50duplicated as a magic number. Both invocation sites hardcode the same value independently; extracting a shared constant (or aSettingsfield) avoids the two copies drifting apart if the limit is tuned later.
src/main.py#L125-L128: source the recursion limit from a shared constant/setting instead of the inline literal50.scripts/test_triage.py#L31-L34: use the same shared constant/setting assrc/main.pyinstead of duplicating the literal50.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/main.py` around lines 125 - 128, Replace the duplicated recursion limit literal with one shared constant or Settings field, and use it in both src/main.py lines 125-128 and scripts/test_triage.py lines 31-34. Update the invocation configuration in each site to reference the shared value so future tuning cannot cause the limits to diverge.src/models/api.py (1)
15-18: 🔒 Security & Privacy | 🔵 Trivial | ⚡ Quick winConsider bounding
reportlength.
reporthasmin_length=1but nomax_length. Since this text flows into LLM extraction and embedding calls, unbounded input allows arbitrarily large/costly requests through an unauthenticated endpoint.🛡️ Suggested bound
report: str = Field( description="Raw bug report text", - min_length=1 + min_length=1, + max_length=20000 )🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/models/api.py` around lines 15 - 18, Update the `report` field declaration in the model to add an appropriate maximum length alongside `min_length=1`, using the project’s established request-size or text-length limit if available. Preserve the existing raw-text description and minimum-length validation.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@scripts/init.sql`:
- Around line 21-30: Add a versioned upgrade migration for existing
issue_embeddings tables that converts the embedding column from 3072 to 1536
dimensions, re-embeds or backfills all existing rows with the new dimension, and
replaces the existing same-named IVFFlat index with the HNSW cosine index. Keep
fresh-database initialization unchanged while ensuring the migration is safe to
rerun.
In `@src/graph/checkpointer.py`:
- Around line 17-25: Update setup_checkpointer to create a psycopg
ConnectionPool configured from settings.database_url, then construct and
initialize PostgresSaver with that pool instead of using
PostgresSaver.from_conn_string. Preserve the existing global context/instance
assignment, setup call, readiness log, and return behavior, while ensuring the
pool lifecycle is managed by the existing checkpointer context.
In `@src/graph/nodes/duplicate.py`:
- Line 143: Update the duplicate-detection condition in the result-handling
logic to use an inclusive confidence comparison, changing the threshold check in
the is_duplicate path from strictly greater-than to greater-than-or-equal.
Preserve the existing settings.duplicate_confidence_threshold symbol and
surrounding behavior.
- Around line 56-71: Update the duplicate-retrieval flow around the issue loop
to use the stored pgvector embeddings rather than generating an embedding for
every issue on each request. Query the embeddings with the database’s vector
distance operator, rank by similarity, and limit results to 5 candidates;
preserve the existing threshold and candidate fields. Remove per-issue embedding
calls and use the configured embedding dimensions consistently with the stored
column.
- Line 45: Update the threshold defaulting logic in the duplicate node to
distinguish an omitted value from an explicitly provided 0.0; only use
settings.embedding_threshold when threshold is None, while preserving all
caller-supplied numeric values.
In `@src/graph/nodes/gitea.py`:
- Around line 207-221: Update the duplicate-report handling around the comment
construction and gitea_service.add_comment_sync call so the implementation
matches the message: either remove the “closed as duplicate” claim from the
comment, or invoke the appropriate gitea_service close-state operation for
issue_id after linking/commenting. Preserve the existing duplicate comment and
issue identifier flow.
- Around line 91-98: The issue is that issue creation can submit a None title
and dummy label IDs. Update create_bug and create_feature_node to derive a
non-empty fallback title when state["title"] is missing, and replace
GiteaService._get_label_id’s hardcoded 0 with real label lookup/creation so
severity and component labels are sent as valid IDs.
In `@src/graph/nodes/risk_check.py`:
- Around line 12-18: Update SECURITY_KEYWORDS and the matching logic in the
associated risk-check flow to use a compiled, case-insensitive boundary-aware
regular expression, ensuring acronyms and multi-word phrases match as whole
terms rather than substrings. Preserve the existing routing behavior for genuine
security terms while preventing matches inside unrelated words such as
“resource.”
In `@src/graph/nodes/triage.py`:
- Around line 116-133: The ValidationError handling in the fast_triage node
suppresses the workflow RetryPolicy. Remove the in-node ValidationError catch
and fallback return from fast_triage so ValidationError propagates to the
configured RetryPolicy; apply the same change to premium_retry_node, preserving
non-validation error handling and successful state updates.
In `@src/graph/nodes/validate.py`:
- Around line 57-65: Update the fallback result in the validation flow to repair
the title before setting validation_passed to True: generate a fallback title
when it is missing, too short, or too long, and clamp it to the accepted length
range so create_issue_node receives a valid state["title"]. If a valid fallback
cannot be produced, retain validation_passed as False and route the node for
human review.
In `@src/graph/workflow.py`:
- Around line 109-136: Resolve the LangGraph compatibility issue for the
error_handler arguments used by fast_triage, premium_retry, and duplicate_check
in build_graph. Either pin the dependency to LangGraph 1.2 or newer in the
requirements configuration, or remove and replace the error_handler arguments
from these add_node calls while preserving equivalent error handling.
In `@src/main.py`:
- Around line 143-153: Update the HTTPException detail in the triage exception
handler to return a generic client-safe failure message without interpolating
exc. Keep the existing logger.error call unchanged so the raw exception remains
available for diagnostics, and remove the unnecessary str(exc) conversion to
satisfy Ruff RUF010.
- Around line 109-130: Update the /api/triage flow around thread_id and
create_initial_state so callers cannot control checkpoint identity: always
generate a fresh server-side UUID for each new execution, or validate any
supplied resume ID against authenticated ownership before using it in the
PostgresSaver configuration. Ensure the validated/generated ID is the only value
passed to both the initial state and _compiled_graph.ainvoke.
In `@src/services/gitea_service.py`:
- Around line 148-178: The create_issue_sync flow lacks replay protection,
allowing duplicate Gitea issues when checkpointed nodes are re-executed. Update
the create_bug/create_feature node logic to return early when gitea_issue_url is
already present, and persist the created issue URL or number in state
immediately after create_issue_sync succeeds so resumed executions reuse it
instead of issuing another POST.
---
Nitpick comments:
In `@src/graph/nodes/triage.py`:
- Around line 41-62: The premium retry path in _build_premium_prompt should use
ChatPromptTemplate consistently with FAST_TRIAGE_PROMPT and reuse the same
system extraction instructions. Preserve the existing retry-specific report,
previous-attempt details, error feedback, and correction focus as the human
prompt content, then return the formatted prompt in the same flattened form
expected by the caller.
In `@src/graph/workflow.py`:
- Around line 41-53: Update route_validation to use settings.max_retries instead
of the hardcoded retry_count >= 2 threshold, matching route_confidence and
validate_node’s fallback retry policy so the configured retry budget
consistently controls routing.
In `@src/main.py`:
- Around line 159-164: Update the uvicorn.run configuration in the __main__
entrypoint to set reload based on settings.environment, enabling it only for
development and disabling it for production or other environments; keep the
existing host, port, and logging configuration unchanged.
- Around line 125-128: Replace the duplicated recursion limit literal with one
shared constant or Settings field, and use it in both src/main.py lines 125-128
and scripts/test_triage.py lines 31-34. Update the invocation configuration in
each site to reference the shared value so future tuning cannot cause the limits
to diverge.
In `@src/models/api.py`:
- Around line 15-18: Update the `report` field declaration in the model to add
an appropriate maximum length alongside `min_length=1`, using the project’s
established request-size or text-length limit if available. Preserve the
existing raw-text description and minimum-length validation.
In `@src/services/gitea_service.py`:
- Around line 148-221: Consolidate the duplicated synchronous HTTP client setup
used by create_issue_sync, add_comment_sync, and list_issues_sync into one
lazily initialized instance-level httpx.Client, configured with the async
client’s 30-second timeout. Initialize the client reference in __init__, reuse
it across calls, and close it in close() alongside existing resources.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: dd8000d1-a8aa-4975-bca5-5f7aabf96b94
📒 Files selected for processing (22)
.cursor/agents/phase-orchestrator/agent.mdrequirements.txtscripts/init.sqlscripts/test_triage.pysrc/config.pysrc/graph/checkpointer.pysrc/graph/nodes/__init__.pysrc/graph/nodes/duplicate.pysrc/graph/nodes/gitea.pysrc/graph/nodes/preprocess.pysrc/graph/nodes/risk_check.pysrc/graph/nodes/triage.pysrc/graph/nodes/validate.pysrc/graph/state.pysrc/graph/workflow.pysrc/main.pysrc/models/api.pysrc/services/embedding_service.pysrc/services/gitea_service.pytests/conftest.pytests/unit/test_nodes.pytests/unit/test_utils.py
| embedding vector(1536), -- text-embedding-3-large truncated (ivfflat/hnsw max 2000 dims) | ||
| stacktrace_hash TEXT, | ||
| created_at TIMESTAMP DEFAULT NOW(), | ||
| updated_at TIMESTAMP DEFAULT NOW() | ||
| ); | ||
|
|
||
| -- Create index for fast similarity search | ||
| -- Create index for fast similarity search (HNSW supports up to 2000 dims) | ||
| CREATE INDEX IF NOT EXISTS issue_embeddings_embedding_idx | ||
| ON issue_embeddings | ||
| USING ivfflat (embedding vector_cosine_ops) | ||
| WITH (lists = 100); | ||
| USING hnsw (embedding vector_cosine_ops); |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
Add an upgrade migration for existing databases.
CREATE TABLE IF NOT EXISTS leaves an existing vector(3072) column unchanged, so the newly configured 1536-dimensional embeddings will fail to persist after upgrade. The existing same-named IVFFlat index also will not be replaced by this HNSW definition. Add a versioned migration that re-embeds/backfills existing rows and replaces the index.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@scripts/init.sql` around lines 21 - 30, Add a versioned upgrade migration for
existing issue_embeddings tables that converts the embedding column from 3072 to
1536 dimensions, re-embeds or backfills all existing rows with the new
dimension, and replaces the existing same-named IVFFlat index with the HNSW
cosine index. Keep fresh-database initialization unchanged while ensuring the
migration is safe to rerun.
| def setup_checkpointer() -> PostgresSaver: | ||
| """Create and initialize PostgresSaver with connection string from settings.""" | ||
| global _checkpointer_ctx, _checkpointer | ||
|
|
||
| _checkpointer_ctx = PostgresSaver.from_conn_string(settings.database_url) | ||
| _checkpointer = _checkpointer_ctx.__enter__() | ||
| _checkpointer.setup() | ||
| logger.info("checkpointer_ready", backend="postgres") | ||
| return _checkpointer |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🌐 Web query:
langgraph-checkpoint-postgres PostgresSaver ConnectionPool vs from_conn_string production
💡 Result:
For production environments, using a ConnectionPool is the recommended practice for PostgresSaver, as it provides robust resource management, handles concurrent requests, and prevents connection timeouts often encountered with long-running workflows [1][2][3]. The two methods differ primarily in their scope and production suitability: 1. PostgresSaver.from_conn_string(conn_string) This helper method is convenient for local development, testing, or quick prototyping [4][5]. Internally, it creates and manages its own connection, but it is generally not recommended for high-concurrency production deployments because it does not provide the sophisticated pooling needed to efficiently handle multiple persistent connections [2][6]. 2. PostgresSaver(pool) with ConnectionPool This is the standard approach for production. By passing a psycopg_pool.ConnectionPool instance to the PostgresSaver, you gain control over critical connection parameters such as minimum and maximum pool size, connection lifetime, and idle timeouts [7][1][8]. When implementing ConnectionPool in production: - Ensure autocommit=True and row_factory=dict_row are configured in the pool's kwargs to satisfy PostgresSaver requirements [7][9][10]. - The.setup method must be called once (often via CI/CD or a migration script, not during every application startup) to initialize the necessary database tables [3]. - Avoid using pipeline mode with a ConnectionPool, as it is designed for use only with a single dedicated connection [11][1]. Example Production Configuration: from psycopg.rows import dict_row from psycopg_pool import ConnectionPool from langgraph.checkpoint.postgres import PostgresSaver pool = ConnectionPool( conn_string, min_size=2, max_size=10, max_idle=300.0, max_lifetime=3600.0, kwargs={ "autocommit": True, "row_factory": dict_row, "prepare_threshold": 0 }) checkpointer = PostgresSaver(pool) # checkpointer.setup # Run once as a migration, not in app runtime[3] graph = builder.compile(checkpointer=checkpointer)
Citations:
- 1: https://langchain-ai-langgraph-40.mintlify.app/api/checkpointing/postgres
- 2: https://kb.langchain.com/articles/6253531756-understanding-checkpointers-databases-api-memory-and-ttl
- 3: https://fast.io/resources/langgraph-persistence/
- 4: http://docs.langchain.com/oss/python/langgraph/add-memory
- 5: https://github.com/langchain-ai/langgraph/blob/4a86705b/libs/checkpoint-postgres/README.md
- 6: https://support.langchain.com/articles/6253531756-understanding-checkpointers-databases-api-memory-and-ttl?threadId=285c524c-7b58-44e4-89ab-210e6393cc7e
- 7: https://support.langchain.com/articles/1242226068-how-do-i-configure-checkpointing-in-langgraph
- 8: https://support.langchain.com/articles/1242226068-how-do-i-configure-checkpointing-in-langgraph?threadId=dd4c57d0-e846-4f0d-85ba-edc0fe5cb6b2
- 9: https://github.com/langchain-ai/langgraph/tree/main/libs/checkpoint-postgres
- 10: https://github.com/langchain-ai/langgraph/blob/5931a5f0/libs/checkpoint-postgres/README.md
- 11: https://github.com/langchain-ai/langgraph/blob/main/libs/checkpoint-postgres/langgraph/checkpoint/postgres/__init__.py
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "Files:"
git ls-files | rg '(^|/)(checkpointer\.py|main\.py|pyproject\.toml|requirements|poetry\.lock|uv\.lock)$' || true
echo
echo "checkpointer.py:"
cat -n src/graph/checkpointer.py
echo
echo "main.py relevant checkpointer usages:"
rg -n "_checkpointer|checkpointer|triage|lifespan|PostgresSaver|setup_checkpointer|close_checkpointer" src/main.py || true
echo
echo "Relevant graph/build lines in main.py:"
sed -n '1,240p' src/main.py | cat -n
echo
echo "Dependency declarations mentioning langgraph/psycopg:"
rg -n 'langgraph|postgres|psycopg|psycopg_pool' pyproject.toml poetry.lock uv.lock requirements*.txt setup.py setup.cfg 2>/dev/null || trueRepository: IWill29/bug-triage-langgraph
Length of output: 8242
Use a pool-backed PostgresSaver for concurrent requests
PostgresSaver.from_conn_string creates a saver backed by a single psycopg connection, while this FastAPI app compiles one graph with that checkpointer and each /api/triage request invokes it concurrently. Move to a ConnectionPool-backed saver instead.
♻️ Sketch: pool-backed checkpointer
-from contextlib import AbstractContextManager
-from typing import Optional
-
+from psycopg_pool import ConnectionPool
+from psycopg.rows import dict_row
+from typing import Optional
+
from langgraph.checkpoint.postgres import PostgresSaver
from src.config import settings
from src.utils.logging import logger
_checkpointer_ctx: Optional[AbstractContextManager[PostgresSaver]] = None
_checkpointer: Optional[PostgresSaver] = None
def setup_checkpointer() -> PostgresSaver:
"""Create and initialize PostgresSaver with connection string from settings."""
global _checkpointer_ctx, _checkpointer
+ _pool = ConnectionPool(
+ conninfo=settings.database_url,
+ min_size=2,
+ max_size=16,
+ kwargs={"autocommit": True, "row_factory": dict_row, "prepare_threshold": 0},
+ )
+ _checkpointer = PostgresSaver(_pool)
+ _checkpointer.setup()
+ logger.info("checkpointer_ready", backend="postgres")
+ return _checkpointer
-def setup_checkpointer() -> PostgresSaver:
- """Create and initialize PostgresSaver with connection string from settings."""
- global _checkpointer_ctx, _checkpointer
-
- _checkpointer_ctx = PostgresSaver.from_conn_string(settings.database_url)
- _checkpointer = _checkpointer_ctx.__enter__()
- _checkpointer.setup()
- logger.info("checkpointer_ready", backend="postgres")
- return _checkpointer🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/graph/checkpointer.py` around lines 17 - 25, Update setup_checkpointer to
create a psycopg ConnectionPool configured from settings.database_url, then
construct and initialize PostgresSaver with that pool instead of using
PostgresSaver.from_conn_string. Preserve the existing global context/instance
assignment, setup call, readiness log, and return behavior, while ensuring the
pool lifecycle is managed by the existing checkpointer context.
| threshold: float | None = None, | ||
| ) -> list[dict[str, Any]]: | ||
| """Retrieve top-K similar issues via embedding similarity.""" | ||
| threshold = threshold or settings.embedding_threshold |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
or treats an explicit 0.0 threshold as unset.
Callers passing threshold=0.0 silently get settings.embedding_threshold.
🐛 Proposed fix
- threshold = threshold or settings.embedding_threshold
+ threshold = settings.embedding_threshold if threshold is None else threshold📝 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.
| threshold = threshold or settings.embedding_threshold | |
| threshold = settings.embedding_threshold if threshold is None else threshold |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/graph/nodes/duplicate.py` at line 45, Update the threshold defaulting
logic in the duplicate node to distinguish an omitted value from an explicitly
provided 0.0; only use settings.embedding_threshold when threshold is None,
while preserving all caller-supplied numeric values.
| query_embedding = embedding_service.generate_embedding(report[:4000]) | ||
| candidates: list[dict[str, Any]] = [] | ||
|
|
||
| for issue in issues: | ||
| text = f"{issue.get('title', '')} {issue.get('body', '')}"[:4000] | ||
| if not text.strip(): | ||
| continue | ||
| issue_embedding = embedding_service.generate_embedding(text) | ||
| score = embedding_service.cosine_similarity(query_embedding, issue_embedding) | ||
| if score >= threshold: | ||
| candidates.append({ | ||
| "id": issue["number"], | ||
| "title": issue.get("title", ""), | ||
| "description": issue.get("body", ""), | ||
| "score": score, | ||
| }) |
There was a problem hiding this comment.
🚀 Performance & Scalability | 🟠 Major | 🏗️ Heavy lift
Re-embedding every open issue on every request; stored pgvector embeddings unused.
This makes up to 101 OpenAI embedding calls per triage run (1 query + 1 per listed issue), all inside a blocking graph node — high latency and cost that grows linearly with open issues. This PR adds a pgvector column and embedding_dimensions config; duplicate retrieval should query stored embeddings with a <=>/<-> similarity search and LIMIT 5 instead. If the per-issue path must stay for now, at minimum batch via embed_documents rather than one embed_query per issue.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/graph/nodes/duplicate.py` around lines 56 - 71, Update the
duplicate-retrieval flow around the issue loop to use the stored pgvector
embeddings rather than generating an embedding for every issue on each request.
Query the embeddings with the database’s vector distance operator, rank by
similarity, and limit results to 5 candidates; preserve the existing threshold
and candidate fields. Remove per-issue embedding calls and use the configured
embedding dimensions consistently with the stored column.
| ) | ||
| continue | ||
|
|
||
| if result.is_duplicate and result.confidence > settings.duplicate_confidence_threshold: |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Threshold comparison is exclusive here but inclusive elsewhere.
result.confidence > settings.duplicate_confidence_threshold rejects an exact-threshold match, while score >= threshold at Line 65 and confidence < confidence_threshold in src/graph/nodes/triage.py treat the boundary as passing. Use >= for consistency.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/graph/nodes/duplicate.py` at line 143, Update the duplicate-detection
condition in the result-handling logic to use an inclusive confidence
comparison, changing the threshold check in the is_duplicate path from strictly
greater-than to greater-than-or-equal. Preserve the existing
settings.duplicate_confidence_threshold symbol and surrounding behavior.
| return { | ||
| "severity": "medium", | ||
| "components": components or ["unknown"], | ||
| "needs_human_review": True, | ||
| "validation_passed": True, | ||
| "processing_warnings": [ | ||
| f"Applied fallback defaults after {retry_count} retries" | ||
| ], | ||
| "validation_errors": [{"errors": errors, "node": "validate"}], |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Repair the title before declaring fallback validation successful.
Line 61 marks validation as passed while preserving a missing, short, or overlong title. The workflow can then reach create_issue_node, which sends state["title"] to Gitea. Generate and clamp a valid fallback title, or keep validation failed and route to review.
Proposed fix
if errors and retry_count >= 2:
+ fallback_title = (title or "Bug report requires review").strip()
+ if len(fallback_title) < 10:
+ fallback_title = "Bug report requires review"
+ fallback_title = fallback_title[:100]
+
return {
+ "title": fallback_title,
"severity": "medium",
"components": components or ["unknown"],📝 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.
| return { | |
| "severity": "medium", | |
| "components": components or ["unknown"], | |
| "needs_human_review": True, | |
| "validation_passed": True, | |
| "processing_warnings": [ | |
| f"Applied fallback defaults after {retry_count} retries" | |
| ], | |
| "validation_errors": [{"errors": errors, "node": "validate"}], | |
| fallback_title = (title or "Bug report requires review").strip() | |
| if len(fallback_title) < 10: | |
| fallback_title = "Bug report requires review" | |
| fallback_title = fallback_title[:100] | |
| return { | |
| "title": fallback_title, | |
| "severity": "medium", | |
| "components": components or ["unknown"], | |
| "needs_human_review": True, | |
| "validation_passed": True, | |
| "processing_warnings": [ | |
| f"Applied fallback defaults after {retry_count} retries" | |
| ], | |
| "validation_errors": [{"errors": errors, "node": "validate"}], |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/graph/nodes/validate.py` around lines 57 - 65, Update the fallback result
in the validation flow to repair the title before setting validation_passed to
True: generate a fallback title when it is missing, too short, or too long, and
clamp it to the accepted length range so create_issue_node receives a valid
state["title"]. If a valid fallback cannot be produced, retain validation_passed
as False and route the node for human review.
| graph.add_node( | ||
| "fast_triage", | ||
| fast_triage_node, | ||
| retry_policy=RetryPolicy( | ||
| retry_on=ValidationError, | ||
| max_attempts=3, | ||
| initial_interval=0.5, | ||
| backoff_factor=2.0, | ||
| ), | ||
| error_handler=handle_triage_error, | ||
| ) | ||
| graph.add_node( | ||
| "premium_retry", | ||
| premium_retry_node, | ||
| retry_policy=RetryPolicy( | ||
| retry_on=ValidationError, | ||
| max_attempts=3, | ||
| initial_interval=0.5, | ||
| backoff_factor=2.0, | ||
| ), | ||
| error_handler=handle_triage_error, | ||
| ) | ||
| graph.add_node("validate", validate_node) | ||
| graph.add_node( | ||
| "duplicate_check", | ||
| duplicate_check_node, | ||
| error_handler=handle_duplicate_error, | ||
| ) |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
python -c "import langgraph, inspect; from langgraph.graph import StateGraph; print(langgraph.__version__); print(inspect.signature(StateGraph.add_node))" 2>/dev/null
rg -n 'langgraph' requirements.txtRepository: IWill29/bug-triage-langgraph
Length of output: 225
🌐 Web query:
LangGraph StateGraph add_node error_handler parameter
💡 Result:
The error_handler parameter in LangGraph's StateGraph.add_node method allows you to define a node-level error handler that executes if a node fails after all retry attempts (as defined by the retry_policy) have been exhausted [1][2]. This feature requires LangGraph version 1.2 or higher [1][2]. Key aspects of the error_handler parameter include: 1. Functionality: It provides a mechanism for graceful recovery or compensation flows (e.g., the Saga pattern) by allowing the handler to return a Command object to update the state or route execution to a different node [1][3][2]. 2. Context Injection: The error handler function can accept an optional argument typed as NodeError (from langgraph.errors) [3][4]. This object contains the name of the failed node and the exception that caused the failure, providing context for the recovery logic [3][4]. Example: def handler(state: State, error: NodeError) -> Command: return Command(update={"status": f"recovered from {error.node}: {error.error}"}) 3. Execution Logic: The handler runs only after the node-level retry_policy is exhausted [1][2]. If no retry policy is configured, the handler executes immediately upon the first failure [2]. 4. Global Defaults: You can configure default error handlers for all nodes in a graph using StateGraph.set_node_defaults, which helps avoid repeating the error_handler parameter on every add_node call [5][2]. Per-node values passed directly to add_node will override these graph-wide defaults [5][2]. 5. Limitations: Error handlers are not invoked if an error occurs within an error handler itself, as this would cause the run to fail [5]. Additionally, error handlers are decoupled from retry policies, allowing you to manage retries and compensation independently [2].
Citations:
- 1: https://docs.langchain.com/oss/python/langgraph/use-graph-api
- 2: https://docs.langchain.com/oss/python/langgraph/fault-tolerance
- 3: feat(langgraph): add node-level error handlers langchain-ai/langgraph#7233
- 4: https://reference.langchain.com/python/langgraph/errors/NodeError
- 5: feat(langgraph): add set_node_defaults() to StateGraph langchain-ai/langgraph#7747
🏁 Script executed:
#!/bin/bash
set -e
python3 - <<'PY'
import inspect
from langgraph.graph import StateGraph
print("available")
sig = inspect.signature(StateGraph.add_node)
print(sig)
print("keywords", sorted(sig.parameters))
# Probe whether this installed package accepts or rejects error_handler as unknown kwarg
# without importing repository code: create a no-op State and node, then try add_node.
class State:
pass
graph = StateGraph(State)
def node(state):
return {}
try:
graph.add_node("test", node, error_handler=lambda *a: None)
print("accepted_error_handler=True")
except TypeError as exc:
print("accepted_error_handler=False")
print("TypeError:", str(exc))
except Exception as exc:
print("unexpected_error", type(exc).__name__, str(exc))
PY
# Inspect repository constraints/imports and the relevant workflow shape without executing repo code.
printf '\n--- workflow lines 1-170 ---\n'
sed -n '1,170p' src/graph/workflow.py 2>/dev/null || true
printf '\n--- error_handler occurrences ---\n'
rg -n "error_handler|add_node|validate_graph|compile|build_graph" src scripts tests requirements.txt pyproject.toml setup.cfg 2>/dev/null || trueRepository: IWill29/bug-triage-langgraph
Length of output: 288
🌐 Web query:
LangGraph 0.2.0 StateGraph add_node parameters retry_policy error_handler
💡 Result:
In LangGraph, the add_node method on StateGraph allows you to configure fault tolerance features, specifically retry policies and error handlers, to manage node execution failures [1][2][3]. Retry Policy and Error Handler Configuration 1. retry_policy: You can pass a RetryPolicy object (or a sequence of them) to the retry_policy parameter in add_node [1][3]. This policy determines if and how a node should be retried after an exception [2][4]. If a sequence is provided, the first matching policy is applied [5][6]. 2. error_handler: The error_handler parameter allows you to register a function that executes only after a node has failed and all associated retry policies have been exhausted [2][7][4]. This handler receives the current state and a NodeError object containing failure context, allowing for recovery flows (such as Saga patterns) via the Command object [2][3][7]. Note that node-level error handlers require langgraph>=1.2 [2][3]. Key Parameters for add_node According to the API, the signature for add_node includes [1][5]: - node: The name of the node (str) or the function/runnable to be executed. - action: The function or runnable associated with the node (required if node is a string). - retry_policy: A RetryPolicy or sequence of policies for automatic retries [1][2]. - error_handler: A function to execute after retries are exhausted [1][2][7]. - input_schema: The input schema for the node (defaults to the graph's state schema) [1][5]. - cache_policy: A CachePolicy for the node [1][5]. - destinations: Used for edgeless graphs with nodes returning Command objects (affects graph rendering only) [1][5]. - timeout: Configures node execution limits (supports float, timedelta, or TimeoutPolicy) [1]. - defer: A boolean to defer execution until the run is about to end [1][5]. - metadata: A dictionary for node-related metadata [1][5]. Graph-Wide Defaults If you want to avoid repeating these configurations for every node, you can use set_node_defaults on your StateGraph to apply retry_policy, error_handler, timeout, or cache_policy settings graph-wide [2][3][4]. Per-node configurations defined in add_node will take precedence over these defaults [3][4].
Citations:
- 1: https://reference.langchain.com/python/langgraph/graph/state/StateGraph/add_node
- 2: https://docs.langchain.com/oss/python/langgraph/fault-tolerance
- 3: https://docs.langchain.com/oss/python/langgraph/use-graph-api
- 4: https://langchain-5e9cc07a.mintlify.app/oss/python/langgraph/fault-tolerance
- 5: https://mintlify.wiki/langchain-ai/langgraph/api/state-graph
- 6: https://github.com/langchain-ai/langgraph/blob/f702729e04dd51b843d257a58f9f5b0181f0be2f/libs/langgraph/langgraph/graph/state.py
- 7: feat(langgraph): add node-level error handlers langchain-ai/langgraph#7233
Pin the LangGraph version to the one that supports error_handler, or remove it from add_node.
error_handler is a supported StateGraph.add_node keyword, but only on LangGraph 1.2+. requirements.txt currently allows langgraph>=0.2.0, so installs of langgraph 0.2.x will reject these add_node(..., error_handler=...) calls with TypeError during build_graph().
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/graph/workflow.py` around lines 109 - 136, Resolve the LangGraph
compatibility issue for the error_handler arguments used by fast_triage,
premium_retry, and duplicate_check in build_graph. Either pin the dependency to
LangGraph 1.2 or newer in the requirements configuration, or remove and replace
the error_handler arguments from these add_node calls while preserving
equivalent error handling.
| if _compiled_graph is None: | ||
| raise HTTPException(status_code=503, detail="Workflow not initialized") | ||
|
|
||
| thread_id = request.thread_id or str(uuid.uuid4()) | ||
|
|
||
| logger.info( | ||
| "triage_request_received", | ||
| report_length=len(request.report), | ||
| thread_id=request.thread_id | ||
| thread_id=thread_id, | ||
| ) | ||
|
|
||
| try: | ||
| # TODO: Implement LangGraph workflow invocation | ||
| # For now, return placeholder response | ||
|
|
||
| return TriageResponse( | ||
| status="pending", | ||
| thread_id=request.thread_id or "generated-id", | ||
| issue_url=None, | ||
| title="TODO: Implement workflow", | ||
| severity="medium", | ||
| components=["unknown"], | ||
| confidence=0.0, | ||
| is_duplicate=False, | ||
| needs_human_review=True, | ||
| warnings=["Workflow not yet implemented"] | ||
| initial_state = create_initial_state( | ||
| bug_report_text=request.report, | ||
| thread_id=thread_id, | ||
| ) | ||
|
|
||
| except Exception as e: | ||
| config = { | ||
| "configurable": {"thread_id": thread_id}, | ||
| "recursion_limit": 50, | ||
| } | ||
|
|
||
| result = await _compiled_graph.ainvoke(initial_state, config) |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | 🏗️ Heavy lift
🧩 Analysis chain
🌐 Web query:
When invoking a compiled LangGraph StateGraph with PostgresSaver, does passing a full initial state dict with an existing thread_id resume/overwrite the prior checkpoint state, or start a fresh run?
💡 Result:
When invoking a compiled LangGraph StateGraph with a persistent checkpointer like PostgresSaver, passing an initial state dictionary alongside an existing thread_id does not simply "resume" or "overwrite" the prior state in the way one might expect. Instead, LangGraph treats the new input as an update to the state [1][2]. Specifically: 1. How it works: When you invoke the graph with an existing thread_id, LangGraph retrieves the latest checkpoint from the database as the starting point [3][4]. 2. Input handling: If you pass a non-empty state dictionary as input, LangGraph merges this new input into the existing checkpoint state [3][1]. This often triggers a new run from the entry node (or the last completed state, depending on graph structure), which may lead to unexpected behavior if you intended to perform a "pure" resumption from the last saved interruption point [2]. 3. Recommended approach for resumption: To resume a graph from its last checkpoint (such as after an interrupt), you should generally pass None as the input (or use Command(resume=...) if explicitly resuming from an interrupt) rather than re-passing the original state [5][2]. Passing None ensures that the runtime uses the last saved checkpoint as the sole source of truth and proceeds from the next node, without attempting to re-process initial input [3][2]. If you pass a full new state dict, it is merged into the existing state, which can result in redundant execution or state conflicts [3][1]. Therefore, to effectively resume, omit the initial input and provide only the configuration containing the thread_id [2].
Citations:
- 1: https://theneuralbase.com/langgraph/learn/intermediate/thread-id-isolating-concurrent-conversations/
- 2: https://forum.langchain.com/t/can-we-resume-from-the-checkpoint-and-continue-running-at-the-interruption-point-instead-of-starting-from-the-first-node/1240
- 3: https://theneuralbase.com/langgraph/learn/advanced/asyncpostgressaver-production-checkpointing/
- 4: https://dreaming.press/posts/resume-crashed-langgraph-run-checkpointer-thread-id.html
- 5: https://docs.langchain.com/oss/python/langgraph/graph-api
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== files =="
git ls-files | sed -n '1,200p'
echo "== locate main.py =="
fd -a 'main.py$' . | sed -n '1,50p'
echo "== src/main.py outline/size =="
if [ -f src/main.py ]; then
wc -l src/main.py
ast-grep outline src/main.py --view condensed || true
echo "== src/main.py relevant lines 1,180 =="
cat -n src/main.py | sed -n '1,180p'
fi
echo "== search thread_id / create_initial_state / PostgresSaver =="
rg -n "thread_id|PostgresSaver|create_initial_state|checkpointer|_compiled_graph|ainvoke|update_state|get_state" .Repository: IWill29/bug-triage-langgraph
Length of output: 16469
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== src/main.py auth/dependencies =="
cat -n src/main.py | sed -n '1,240p'
echo "== src/models/api.py =="
cat -n src/models/api.py | sed -n '1,120p'
echo "== auth/session headers in repo =="
rg -n "Authorization|Bearer|API-Key|X-.*Key|Depends\\(|Security|API|APIKey|thread_id" src tests scripts || true
echo "== graph/state relevant =="
wc -l src/graph/state.py src/graph/workflow.py
cat -n src/graph/state.py | sed -n '1,120p'
echo "== workflow invoke/update/delete relevant =="
cat -n src/graph/workflow.py | sed -n '1,180p'
echo "== deterministic path check: route is unauthenticated? =="
python3 - <<'PY'
from pathlib import Path
p = Path('src/main.py')
text = p.read_text()
checks = {
"post_triage_uses_request_thread_id": 'thread_id = request.thread_id or str(uuid.uuid4())' in text,
"post_triage_passes_client_thread_id_to_invoke": '"configurable": {"thread_id": thread_id}' in text,
"uses_persistent_postgres_checkpointer": "PostgresSaver.from_conn_string(settings.database_url)" in text,
"health_root_no_auth_decorators": "`@app.get`(\"/health\")\nasync def health_check()" in text and "Depends(" in text[text.find('`@app.get`("/health")'):text.find('async def health_check')+800] == False,
"root_no_auth_decorators": "`@app.get`(\"/\")\nasync def root()" in text and "Depends(" in text[text.find('`@app.get`("/")'):text.find('async def root')+800] == False,
}
for k, v in checks.items():
print(f"{k}: {v}")
PYRepository: IWill29/bug-triage-langgraph
Length of output: 22859
Don’t accept client-supplied checkpoint IDs.
thread_id is attacker-controllable on /api/triage and is passed directly into the PostgresSaver configuration key. Any caller can guess or reuse another thread ID and merge state into or extend another request's persisted checkpoint chain. Generate thread_id server-side for new executions, or require authenticated ownership before allowing resume IDs.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/main.py` around lines 109 - 130, Update the /api/triage flow around
thread_id and create_initial_state so callers cannot control checkpoint
identity: always generate a fresh server-side UUID for each new execution, or
validate any supplied resume ID against authenticated ownership before using it
in the PostgresSaver configuration. Ensure the validated/generated ID is the
only value passed to both the initial state and _compiled_graph.ainvoke.
| except Exception as exc: | ||
| logger.error( | ||
| "triage_failed", | ||
| error=str(e), | ||
| error_type=type(e).__name__ | ||
| error=str(exc), | ||
| error_type=type(exc).__name__, | ||
| thread_id=thread_id, | ||
| ) | ||
| raise HTTPException( | ||
| status_code=500, | ||
| detail=f"Triage processing failed: {str(e)}" | ||
| ) | ||
| detail=f"Triage processing failed: {str(exc)}", | ||
| ) from exc |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
Raw exception message leaked to API clients.
detail=f"Triage processing failed: {str(exc)}" returns the internal exception text (potentially including DB/psycopg errors, file paths, or LLM provider errors) directly to the caller. Return a generic message and rely on the existing logger.error(...) call (already capturing error=str(exc)) for diagnostics.
Also addresses the Ruff RUF010 hint (explicit conversion flag instead of str()).
🛡️ Proposed fix
except Exception as exc:
logger.error(
"triage_failed",
error=str(exc),
error_type=type(exc).__name__,
thread_id=thread_id,
)
raise HTTPException(
status_code=500,
- detail=f"Triage processing failed: {str(exc)}",
+ detail="Triage processing failed. See server logs for details.",
) from exc📝 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.
| except Exception as exc: | |
| logger.error( | |
| "triage_failed", | |
| error=str(e), | |
| error_type=type(e).__name__ | |
| error=str(exc), | |
| error_type=type(exc).__name__, | |
| thread_id=thread_id, | |
| ) | |
| raise HTTPException( | |
| status_code=500, | |
| detail=f"Triage processing failed: {str(e)}" | |
| ) | |
| detail=f"Triage processing failed: {str(exc)}", | |
| ) from exc | |
| except Exception as exc: | |
| logger.error( | |
| "triage_failed", | |
| error=str(exc), | |
| error_type=type(exc).__name__, | |
| thread_id=thread_id, | |
| ) | |
| raise HTTPException( | |
| status_code=500, | |
| detail="Triage processing failed. See server logs for details.", | |
| ) from exc |
🧰 Tools
🪛 Ruff (0.16.0)
[warning] 152-152: Use explicit conversion flag
Replace with conversion flag
(RUF010)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/main.py` around lines 143 - 153, Update the HTTPException detail in the
triage exception handler to return a generic client-safe failure message without
interpolating exc. Keep the existing logger.error call unchanged so the raw
exception remains available for diagnostics, and remove the unnecessary str(exc)
conversion to satisfy Ruff RUF010.
Source: Linters/SAST tools
| def create_issue_sync( | ||
| self, | ||
| title: str, | ||
| body: str, | ||
| labels: Optional[List[str]] = None, | ||
| ) -> Dict[str, Any]: | ||
| """Synchronous issue creation for LangGraph sync nodes.""" | ||
| logger.info( | ||
| "gitea_create_issue_sync", | ||
| title=title, | ||
| labels=labels or [], | ||
| ) | ||
|
|
||
| url = f"/api/v1/repos/{self.repo_owner}/{self.repo_name}/issues" | ||
| payload = { | ||
| "title": title, | ||
| "body": body, | ||
| "labels": [self._get_label_id(label) for label in (labels or [])], | ||
| } | ||
|
|
||
| with httpx.Client( | ||
| base_url=self.base_url, | ||
| headers={ | ||
| "Authorization": f"token {self.token}", | ||
| "Content-Type": "application/json", | ||
| }, | ||
| timeout=45.0, | ||
| ) as client: | ||
| response = client.post(url, json=payload) | ||
| response.raise_for_status() | ||
| return response.json() |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy lift
Non-idempotent write with no replay guard.
create_issue_sync is invoked from the create_bug/create_feature nodes, which run under PostgresSaver checkpointing. If a thread is resumed or the node is re-executed after a partial failure (e.g. the POST succeeded but the checkpoint write did not), a second Gitea issue is created. Consider recording the created issue number in state before returning, and short-circuiting the node when gitea_issue_url is already present.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/services/gitea_service.py` around lines 148 - 178, The create_issue_sync
flow lacks replay protection, allowing duplicate Gitea issues when checkpointed
nodes are re-executed. Update the create_bug/create_feature node logic to return
early when gitea_issue_url is already present, and persist the created issue URL
or number in state immediately after create_issue_sync succeeds so resumed
executions reuse it instead of issuing another POST.
PostgresSaver does not implement async checkpoint APIs, so ainvoke raised NotImplementedError before any node ran. Use sync invoke via asyncio.to_thread to match sync nodes and the spec checkpoint pattern.
…ation docs. Align agent specs with agent-template, route SonarCloud checks through the orchestrator, and record Phase 2 PR #6 CI/QA status in WORKFLOW.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/main.py`:
- Around line 131-134: Replace the asyncio.to_thread call around
_compiled_graph.invoke with cancellation-safe durable execution: route the
workflow through a durable job model, or introduce pre-committed execution IDs
with idempotent server-owned checkpoint and Gitea writes. Ensure cancelling the
request cannot leave an untracked LangGraph step mutating state and that retries
using the same thread ID cannot duplicate or conflict with those mutations.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
| # PostgresSaver is sync-only; sync nodes use invoke (not ainvoke). | ||
| result = await asyncio.to_thread( | ||
| _compiled_graph.invoke, initial_state, config | ||
| ) |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy lift
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== files =="
git ls-files | sed -n '1,120p'
echo "== locate main.py =="
fd -a 'main\.py$' . || true
echo "== outline src/main.py if present =="
if [ -f src/main.py ]; then
wc -l src/main.py
ast-grep outline src/main.py --view expanded | sed -n '1,220p' || true
echo "== relevant section =="
sed -n '100,180p' src/main.py
fi
echo "== relevant symbols/usages =="
rg -n "PostgresSaver|compiled_graph|asyncio\.to_thread|invoke\(|timeout|config|idempotenc|execution|cancel|Gitea|gitea" src || trueRepository: IWill29/bug-triage-langgraph
Length of output: 10053
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== git diff summary =="
git diff --stat || true
echo "== changed files in src/main.py =="
git diff -- src/main.py | sed -n '1,260p' || true
echo "== inspect lockfile python version hints =="
for f in pyproject.toml requirements.txt Pipfile setup.py setup.cfg poetry.lock uv.lock; do
[ -f "$f" ] && { echo "--- $f"; sed -n '1,160p' "$f"; }
doneRepository: IWill29/bug-triage-langgraph
Length of output: 937
Make the offloaded workflow cancellation-safe.
asyncio.to_thread runs _compiled_graph.invoke(...) synchronously, so cancelling the request task does not stop an ongoing LangGraph step. That can leave a checkpoint update or Gitea issue/comment mutation running and later duplicate/conflict with a retried request using the same thread ID. Route this through a durable job model, or use pre-committed execution IDs and idempotent server-owned writes.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/main.py` around lines 131 - 134, Replace the asyncio.to_thread call
around _compiled_graph.invoke with cancellation-safe durable execution: route
the workflow through a durable job model, or introduce pre-committed execution
IDs with idempotent server-owned checkpoint and Gitea writes. Ensure cancelling
the request cannot leave an untracked LangGraph step mutating state and that
retries using the same thread ID cannot duplicate or conflict with those
mutations.
Source: MCP tools
|
There was a problem hiding this comment.
Actionable comments posted: 7
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.cursor/agents/bug-triage-reviewer/agent.md:
- Line 80: Add “Empty input” as a required validation case in the Set B
validation checklist, alongside the existing B1–B8 cases.
In @.cursor/agents/code-auditor/agent.md:
- Around line 36-40: Update the scoring denominator and workflow checklist in
the agent instructions to match build_graph(): score all registered nodes and
cover the terminal routes create_bug, create_feature, comment_duplicate, and
human_review. Replace non-existent sequence terms such as confidence_gate and
create_issue with the actual fast_triage confidence routing and registered node
names, ensuring create_feature is explicitly included.
In @.cursor/agents/phase-orchestrator/agent.md:
- Line 157: Standardize Set B reporting on the orchestrator’s canonical
seven-sample denominator. Update the final PR report in
.cursor/agents/phase-orchestrator/agent.md (lines 157-157), the Set B reporting
template in .cursor/agents/code-auditor/agent.md (lines 101-110), and the QA
template in .cursor/agents/qa-tester/agent.md (lines 138-147) to use seven;
track Empty separately from the denominator where applicable.
- Around line 118-123: Update the merge workflow around the `gh pr list --head`
command to use the exact known phase branch value instead of the `phase-{N}-*`
glob. If the exact branch is unavailable, list the relevant PRs and filter their
JSON output to identify the matching phase branch before running `gh pr merge`.
In @.cursor/agents/qa-tester/agent.md:
- Around line 182-188: Update the passed condition in the Decision rules table
to require every mandatory functional sample (B1–B8) and required edge case
(E1–E8), while preserving the existing B3, empty, B5, and mode requirements. If
any required case fails without meeting the failed criteria, classify the result
as partial rather than passed.
In @.cursor/agents/spec-architect/agent.md:
- Around line 53-57: Add the required stacktrace-hash stage to all
duplicate-detection guidance: in .cursor/agents/spec-architect/agent.md lines
53-57, require exact-hash matching or candidate filtering before embedding and
LLM comparison; in .cursor/agents/bug-fixer/agent.md line 46, include the
exact-hash path in the B5 fix mapping; and in
.cursor/agents/bug-triage-reviewer/agent.md line 50, add stacktrace-hash
verification to the audit checklist.
In `@WORKFLOW.md`:
- Around line 64-70: Clarify the Phase 2 merge guidance in WORKFLOW.md to state
that an explicit merge is risk acceptance only and does not complete blocked QA.
Update the phase completion logic in .cursor/agents/phase-orchestrator/agent.md
around the merge flow so the phase is marked complete only after QA completes,
or remains explicitly blocked after merging.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: d8c3e2d8-00a4-4cdc-b5cf-54c6bccefc66
📒 Files selected for processing (9)
.cursor/agents/agent-template.md.cursor/agents/bug-fixer/agent.md.cursor/agents/bug-triage-reviewer/agent.md.cursor/agents/code-auditor/agent.md.cursor/agents/phase-orchestrator/agent.md.cursor/agents/qa-tester/agent.md.cursor/agents/spec-architect/agent.md.cursor/rules/sonarqube.mdcWORKFLOW.md
| 4. **Categorize issues** — 🔴 critical (demo blockers), 🟡 high, 🟢 medium, ⚪ low. | ||
|
|
||
| Validate implementation includes: | ||
| 5. **Set B validation** — B1 medium extract; B3 low confidence; B4 severity override; B5 EXIST-1 duplicate; B6 feature flag; B7 primary; B8 log cleanup. |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Check empty input in Part B.
Part A and QA define empty input as the eighth required case. The Part B checklist omits it. Add Empty input to the required validation list.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In @.cursor/agents/bug-triage-reviewer/agent.md at line 80, Add “Empty input” as
a required validation case in the Set B validation checklist, alongside the
existing B1–B8 cases.
| 1. **Spec alignment** — verify each major spec component in code: | ||
| - **State schema:** fields, types, `Annotated[..., operator.add]` reducers match spec | ||
| - **Node sequence:** preprocess → risk_check → fast_triage → confidence_gate → premium_retry → validate → duplicate_check → create_issue | ||
| - **Routing:** confidence gate 0.70, risk escalation, duplicate → create OR comment, `Literal` hints, fallbacks | ||
|
|
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
rg -n 'add_node|add_conditional_edges|human_review|gitea|create_issue|comment' \
src/graph/workflow.py src/graph/nodesRepository: IWill29/bug-triage-langgraph
Length of output: 5855
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== workflow outline =="
ast-grep outline src/graph/workflow.py --view expanded || true
echo "== workflow relevant lines =="
sed -n '1,210p' src/graph/workflow.py | cat -n
echo "== node registrations and conditions in workflow =="
python3 - <<'PY'
from pathlib import Path
p = Path('src/graph/workflow.py')
text = p.read_text()
for name in ['graph.add_node', 'graph.add_conditional_edges', 'graph.add_edge']:
print(f'-- {name} --')
start = text.find(name)
while start != -1:
# find end at next name or EOF
end = text.find('graph.add', start + len(name))
if end == -1:
end = len(text)
print(text[start:end].strip())
start = text.find(name, start + len(name))
PY
echo "== agent excerpt =="
sed -n '1,80p' .cursor/agents/code-auditor/agent.md | cat -nRepository: IWill29/bug-triage-langgraph
Length of output: 13772
Align the score denominator and node checklist with the workflow graph.
.cursor/agents/code-auditor/agent.md scores X/8, but build_graph() has ten nodes and four terminal routes: create_bug, create_feature, comment_duplicate, and human_review. The “Node sequence” line also uses non-existent terms like confidence_gate and create_issue instead of the registered fast_triage confidence routing and the create_bug / create_feature nodes. Update the checklist to cover every registered node and endpoint, including create_feature.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In @.cursor/agents/code-auditor/agent.md around lines 36 - 40, Update the
scoring denominator and workflow checklist in the agent instructions to match
build_graph(): score all registered nodes and cover the terminal routes
create_bug, create_feature, comment_duplicate, and human_review. Replace
non-existent sequence terms such as confidence_gate and create_issue with the
actual fast_triage confidence routing and registered node names, ensuring
create_feature is explicitly included.
| When user says `merge` / `merge pr`: | ||
| ```bash | ||
| git diff main -- spec.md | ||
| ``` | ||
|
|
||
| - **No diff** → skip `@spec-architect`, note in report. | ||
| - **Has diff** → launch `@spec-architect` via Task; require score ≥ acceptable threshold and no unresolved critical gaps before continuing. | ||
|
|
||
| ### 4. Code Audit (always) | ||
|
|
||
| Launch `@code-auditor` via Task with: | ||
|
|
||
| ``` | ||
| Audit phase-{N} implementation on branch phase-{N}-{description}. | ||
| Compare against spec.md. Report: pass | warnings | critical with file:line refs. | ||
| gh pr list --head phase-{N}-* | ||
| gh pr merge {number} --squash | ||
| git checkout main && git pull origin main | ||
| ``` |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
gh help pr list | rg -n -- '--head|glob|wildcard'
gh pr list --state open --head phase-2-workflow-nodesRepository: IWill29/bug-triage-langgraph
Length of output: 371
🌐 Web query:
GitHub CLI gh pr list --head wildcard branch glob
💡 Result:
The GitHub CLI command gh pr list does not support wildcard characters or glob patterns for the --head flag [1][2]. The --head option is designed to filter by an exact head branch name [1][3]. It specifically does not support the : syntax often used in other GitHub contexts, nor does it support partial matches or pattern matching [1][4][2]. If you need to filter pull requests based on partial branch names or complex patterns, the recommended approach is to use client-side filtering with the --json and --jq flags [3]. You can retrieve a broader set of pull requests and then refine them using jq [3]. For example: gh pr list --json headRefName,title,number,url --jq 'map(select(.headRefName | startswith("feature/")))' Alternatively, if you require server-side filtering, you can use the --search flag, which leverages GitHub's search syntax [5][6]. While this also does not support standard shell globs, it allows for some flexible search queries, though it is generally best suited for exact branch matches or other metadata filters [3][5].
Citations:
- 1: https://cli.github.com/manual/gh_pr_list
- 2: https://cli.github.com/manual/gh_help_reference
- 3: PR search by multiple exact branch names cli/cli#10816
- 4: Option to search for a branch name in command "gh pr list" cli/cli#2977
- 5: https://docs.github.com/en/issues/tracking-your-work-with-issues/using-issues/filtering-and-searching-issues-and-pull-requests?tool=cli
- 6: https://man.archlinux.org/man/gh-pr-list.1.en.raw
🌐 Web query:
GitHub CLI pr list --head wildcard branch glob
💡 Result:
The GitHub CLI (gh pr list) does not natively support glob patterns or wildcards for the --head flag [1][2][3]. The --head flag performs an exact match for the head branch name [1][4]. Furthermore, official documentation explicitly notes that the : syntax (often used for qualified references like owner:branch) is not supported by this flag [1][5][2]. To achieve filtering with patterns, wildcards, or complex matching logic, you must perform client-side filtering after retrieving the pull request data. This is commonly done by combining the --json flag to export the necessary fields and the jq tool to process the output [4]. Example approach using jq for pattern matching: gh pr list --json number,title,headRefName | jq 'map(select(.headRefName | test("your-pattern-here")))' In this command: 1. --json number,title,headRefName fetches the pull requests with the required fields [1][4]. 2. jq 'map(select(.headRefName | test("...")))' filters the results client-side, where test() supports regular expression patterns [4].
Citations:
- 1: https://cli.github.com/manual/gh_pr_list
- 2: https://man.archlinux.org/man/gh-pr-list.1.en.raw
- 3: https://www.gsp.com/cgi-bin/man.cgi?topic=GH-PR-LIST
- 4: PR search by multiple exact branch names cli/cli#10816
- 5: Add example usage of
--headoption topr listdocs cli/cli#10979
Use an exact branch value in the merge lookup.
gh pr list --head phase-{N}-* does not expand shell globs and --head matches the pull request head branch exactly, so the merge lookup fails unless {N} is replaced with the exact phase branch. Pass the known phase branch or filter JSON output after listing PRs if exact lookup is not available.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In @.cursor/agents/phase-orchestrator/agent.md around lines 118 - 123, Update
the merge workflow around the `gh pr list --head` command to use the exact known
phase branch value instead of the `phase-{N}-*` glob. If the exact branch is
unavailable, list the relevant PRs and filter their JSON output to identify the
matching phase branch before running `gh pr merge`.
| Ensure working tree is clean on feature branch. Push: | ||
| **PR:** [URL] | ||
| **Auditor:** ✅ pass | ⚠️ warnings | ❌ critical — [summary] | ||
| **QA:** ✅ [X/7 Set B] | ⚠️ partial | ❌ failed | **BLOCKED** — [blockers] |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Standardize the Set B denominator across reports.
The orchestrator uses seven Set B samples. The auditor and QA templates use eight and include Empty. Choose one definition and apply it consistently.
.cursor/agents/phase-orchestrator/agent.md#L157-L157: align the final PR report with the canonical denominator..cursor/agents/code-auditor/agent.md#L101-L110: use the same denominator and separate empty-input coverage if needed..cursor/agents/qa-tester/agent.md#L138-L147: use the same denominator and separate empty-input coverage if needed.
📍 Affects 3 files
.cursor/agents/phase-orchestrator/agent.md#L157-L157(this comment).cursor/agents/code-auditor/agent.md#L101-L110.cursor/agents/qa-tester/agent.md#L138-L147
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In @.cursor/agents/phase-orchestrator/agent.md at line 157, Standardize Set B
reporting on the orchestrator’s canonical seven-sample denominator. Update the
final PR report in .cursor/agents/phase-orchestrator/agent.md (lines 157-157),
the Set B reporting template in .cursor/agents/code-auditor/agent.md (lines
101-110), and the QA template in .cursor/agents/qa-tester/agent.md (lines
138-147) to use seven; track Empty separately from the denominator where
applicable.
| ## Decision rules | ||
|
|
||
| --- | ||
| | Outcome | Condition | Action | | ||
| |---------|-----------|--------| | ||
| | ✅ **passed** | Set B attempted; B5 pass; no crash on B3/empty; mode documented | Orchestrator may open PR | | ||
| | ⚠️ **partial** | Non-critical failures or slow perf | Orchestrator opens PR with honest partial status | | ||
| | ❌ **failed** | B5 fail, B3 crash, or critical edge fail | Orchestrator → `@bug-fixer` → retest (max 3 loops) | |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Do not mark QA passed when required samples fail.
The passed rule checks only B5, B3, Empty, and execution mode. It does not require B1, B4, B6, B7, B8, or E1–E8. A report can therefore say passed with failed required cases.
Require all mandatory functional samples and required edge cases for passed. Otherwise return partial.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In @.cursor/agents/qa-tester/agent.md around lines 182 - 188, Update the passed
condition in the Decision rules table to require every mandatory functional
sample (B1–B8) and required edge case (E1–E8), while preserving the existing B3,
empty, B5, and mode requirements. If any required case fails without meeting the
failed criteria, classify the result as partial rather than passed.
| 3. **Validate duplicate detection** — two-stage required: | ||
| - Stage 1: embeddings threshold **0.70–0.75** (NOT 0.85+) | ||
| - Stage 2: LLM semantic comparison **0.80+** | ||
| - Research cited; false-positive mitigation; cost/accuracy tradeoff | ||
| - Anti-patterns: single-stage only, threshold > 0.85, LLM-only |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
Document the stacktrace-hash stage in every duplicate-detection instruction.
The PR objective requires stacktrace hashes, embedding similarity, and LLM comparison. These instructions describe only embeddings followed by LLM comparison.
.cursor/agents/spec-architect/agent.md#L53-L57: require exact-hash matching or candidate filtering before embedding and LLM comparison..cursor/agents/bug-fixer/agent.md#L46-L46: update the B5 fix mapping with the exact-hash path..cursor/agents/bug-triage-reviewer/agent.md#L50-L50: add stacktrace-hash verification to the audit checklist.
📍 Affects 3 files
.cursor/agents/spec-architect/agent.md#L53-L57(this comment).cursor/agents/bug-fixer/agent.md#L46-L46.cursor/agents/bug-triage-reviewer/agent.md#L50-L50
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In @.cursor/agents/spec-architect/agent.md around lines 53 - 57, Add the
required stacktrace-hash stage to all duplicate-detection guidance: in
.cursor/agents/spec-architect/agent.md lines 53-57, require exact-hash matching
or candidate filtering before embedding and LLM comparison; in
.cursor/agents/bug-fixer/agent.md line 46, include the exact-hash path in the B5
fix mapping; and in .cursor/agents/bug-triage-reviewer/agent.md line 50, add
stacktrace-hash verification to the audit checklist.
| - [ ] **Phase 2:** Workflow nodes — **IN PROGRESS** ([PR #6](https://github.com/IWill29/bug-triage-langgraph/pull/6), branch `phase-2-workflow-nodes`) | ||
| - CI: SonarCloud ✅ | unit tests 15/15 ✅ | `/api/triage` invoke fix (`cfc2f6a`) | ||
| - QA: **BLOCKED** — Set B needs real `OPENAI_API_KEY` + `GITEA_TOKEN` + Gitea Set A seed | ||
| - [ ] **Phase 3:** Production hardening (`phase-3-production-hardening`) | ||
| - [ ] **Phase 4:** Testing (`phase-4-testing`) | ||
|
|
||
| After Phase 1 merge, say **`next`** to start Phase 2. | ||
| Phase 2 PR is open — add keys + Gitea setup, then **`next`** to re-run Set B QA, or **`merge`** to land code and defer live QA to Phase 4. |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Align blocked-QA handling with the merge flow.
WORKFLOW.md permits merging while Phase 2 QA is blocked, while the orchestrator merge flow marks the phase complete after merge. This can convert an unverified phase into a completed phase.
WORKFLOW.md#L64-L70: state that explicit merge is risk acceptance and does not complete QA..cursor/agents/phase-orchestrator/agent.md#L116-L124: do not mark the phase complete until QA is complete, or preserve an explicit blocked status after merge.
📍 Affects 2 files
WORKFLOW.md#L64-L70(this comment).cursor/agents/phase-orchestrator/agent.md#L116-L124
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@WORKFLOW.md` around lines 64 - 70, Clarify the Phase 2 merge guidance in
WORKFLOW.md to state that an explicit merge is risk acceptance only and does not
complete blocked QA. Update the phase completion logic in
.cursor/agents/phase-orchestrator/agent.md around the merge flow so the phase is
marked complete only after QA completes, or remains explicitly blocked after
merging.
1 participant
Summary
src/graph/checkpointer.py(not MemorySaver)/api/triageinvokes compiled graph with thread_id checkpointingAgent-generated vs manual changes
Test plan
pytest tests/unit -q— 15 passedbuild_graph().compile())/api/triagesmoke with live servicesKnown TODOs
_get_label_idreturns dummy 0)scripts/validate_duplicate_detection.py)Summary by CodeRabbit