Conversation
📝 WalkthroughWalkthroughAdds a FastAPI web API and static dashboard, SQLAlchemy persistence with Alembic migrations, SCM provider implementations (GitHub, GitLab, Bitbucket), JIRA metrics, collection/scheduler services, CLI commands for collection/serve/dora/ai/survey, Docker containerization, and extensive README and packaging updates. Changes
Sequence DiagramssequenceDiagram
participant User as User/CLI
participant Collector as Collector Service
participant Providers as SCM Providers
participant Store as MetricStore
participant DB as Database
User->>Collector: collect_all(period_start, period_end)
Collector->>Collector: _get_configured_providers()
Collector->>Store: create MetricSnapshot
Store->>DB: insert MetricSnapshot
loop per provider
Collector->>Providers: get_repositories()
Providers-->>Collector: SCMRepository[]
Collector->>Providers: get_pull_requests(since, until)
Providers-->>Collector: SCMPullRequest[]
Collector->>Store: upsert PullRequestMetric
Store->>DB: insert/update PullRequestMetric
Collector->>Providers: get_deployments(since, until)
Providers-->>Collector: SCMDeployment[]
Collector->>Store: add DeploymentMetric
Store->>DB: insert DeploymentMetric
end
Collector->>Store: complete snapshot
Store->>DB: update MetricSnapshot, commit
Collector-->>User: return summary
sequenceDiagram
participant Client as Web Client
participant FastAPI as FastAPI App
participant Lifespan as Lifespan Init
participant Routes as API Routes
participant Store as MetricStore
participant DB as Database
Client->>FastAPI: GET /api/v1/dora
FastAPI->>Lifespan: (startup) init_db()
Lifespan->>DB: create tables
FastAPI->>Routes: route handler get_dora_metrics
Routes->>Store: get_session()
Store->>DB: query PRs, deployments, incidents
DB-->>Store: metric records
Routes->>Services: compute_dora(store, period_start, period_end)
Services->>Store: save DORA snapshot
Store->>DB: insert DORASnapshot, commit
Routes->>Store: close()
Routes-->>Client: JSON DORAResponse
Estimated code review effort🎯 4 (Complex) | ⏱️ ~55 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 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 |
wellcode-ai
bot
added
security-sensitive
🔍 General Code Quality Feedback🔍 Comprehensive Code ReviewConsolidated Feedback
Overall Assessment: This pull request introduces significant enhancements to the Wellcode CLI, including a FastAPI-based web dashboard and various new API endpoints. While the implementation shows promise, there are critical issues related to maintainability, testing, and security that need to be addressed before merging. Critical Issues:
Improvements:
Positive Notes:
Next Steps:
By addressing these critical issues and implementing the suggested improvements, the quality and maintainability of the codebase will significantly enhance, leading to a more robust and secure application. 🤖 Generated by Wellcode.ai |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
Note
Due to the large number of review comments, Critical severity comments were prioritized as inline comments.
🟠 Major comments (31)
Dockerfile-1-18 (1)
1-18:⚠️ Potential issue | 🟠 MajorContainer runs as root — add a non-root user.
Running the application as
rootinside the container widens the attack surface. Create a dedicated user and switch to it beforeCMD.🔒 Proposed fix
RUN pip install --no-cache-dir . +RUN useradd --create-home appuser +USER appuser + ENV WELLCODE_DATA_DIR=/data EXPOSE 8787Note: if the
/datavolume needs to be writable byappuser, ensure the directory ownership is set accordingly (e.g.,RUN mkdir -p /data && chown appuser:appuser /databefore theUSERdirective).🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Dockerfile` around lines 1 - 18, Add a non-root user and switch to it before CMD: create a dedicated user (e.g., appuser) and group, ensure /app and the WELLCODE_DATA_DIR (/data from ENV WELLCODE_DATA_DIR) are created and chowned to that user (and group) so the process can write, then add a USER directive so the final process (invoked by CMD ["wellcode", "serve", ...]) runs as that non-root user; also ensure any build-time steps that require root (apt-get, pip install) remain before switching users and reference WORKDIR /app and the ENV variable when changing ownership.Dockerfile-14-14 (1)
14-14:⚠️ Potential issue | 🟠 Major
WELLCODE_DATA_DIRenvironment variable is not consumed by the application.The Dockerfile sets
WELLCODE_DATA_DIR=/data, but the Python codebase does not reference this variable anywhere. The database engine (src/wellcode_cli/db/engine.py:15) hard-codesDATA_DIR = Path.home() / ".wellcode" / "data", which means the/datavolume mount in docker-compose is ineffective for database persistence. Database files will be written to the container's home directory rather than the mounted volume.Update
engine.pyto respect theWELLCODE_DATA_DIRenvironment variable, or remove it from the Dockerfile if it is not needed.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Dockerfile` at line 14, The Dockerfile ENV WELLCODE_DATA_DIR is unused; update src/wellcode_cli/db/engine.py so DATA_DIR reads os.environ.get("WELLCODE_DATA_DIR") and falls back to Path.home() / ".wellcode" / "data" (preserve current behavior if env var unset), i.e., replace the hard-coded DATA_DIR = Path.home() / ".wellcode" / "data" with logic that respects WELLCODE_DATA_DIR (and ensure conversion to a Path and creation of the directory if missing), or alternatively remove the ENV from the Dockerfile if you prefer not to support an override.src/wellcode_cli/integrations/github/provider.py-111-114 (1)
111-114:⚠️ Potential issue | 🟠 Major
min()on empty sequence raisesValueError.If all reviews have
submitted_atasNone, the generator inmin(...)yields no values andmin()raisesValueError.Proposed fix
first_review_at = None - if reviews: - fr = min(r.submitted_at for r in reviews if r.submitted_at) - first_review_at = fr.replace(tzinfo=timezone.utc) if fr.tzinfo is None else fr + review_dates = [r.submitted_at for r in reviews if r.submitted_at] + if review_dates: + fr = min(review_dates) + first_review_at = fr.replace(tzinfo=timezone.utc) if fr.tzinfo is None else fr🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/wellcode_cli/integrations/github/provider.py` around lines 111 - 114, The block computing first_review_at can raise ValueError because min(...) gets no items when all review.submitted_at are None; update the logic in the provider code that sets first_review_at (look for the reviews variable and the fr assignment) to first filter submitted_at values into a list or iterator (e.g., timestamps = [r.submitted_at for r in reviews if r.submitted_at]) and only call min(...) when that collection is non-empty, otherwise leave first_review_at as None; ensure the timezone handling (fr.replace(tzinfo=timezone.utc) if fr.tzinfo is None else fr) is applied to the min value.src/wellcode_cli/integrations/gitlab/provider.py-79-81 (1)
79-81:⚠️ Potential issue | 🟠 MajorPotential
AttributeErrorifmr.authorisNone.When a GitLab user is deleted or the MR was created by an automated system,
mr.authorcan beNone. Calling.get("username")onNonewill raiseAttributeError. The same issue applies on line 112.Proposed fix
- if author and mr.author.get("username") != author: + mr_author = (mr.author or {}).get("username", "") + if author and mr_author != author: continueAnd on line 112:
- author=mr.author.get("username", ""), + author=mr_author,🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/wellcode_cli/integrations/gitlab/provider.py` around lines 79 - 81, The loop "for mr in mrs" accesses mr.author.get("username") which can raise AttributeError when mr.author is None; update the condition to first ensure mr.author is truthy (e.g., if mr.author and mr.author.get("username") == author) before comparing, and apply the same guard to the similar check around the code referenced at line 112 so any code that uses mr.author.get("username") first verifies mr.author is not None.src/wellcode_cli/integrations/bitbucket/provider.py-60-78 (1)
60-78:⚠️ Potential issue | 🟠 MajorNo pagination — only the first page of repositories is fetched.
The Bitbucket API paginates results. With
pagelen: 100, only the first 100 repos are returned. Any workspace with more repositories will have data silently dropped. The same issue applies toget_pull_requests(line 96–99,pagelen: 50).You need to follow the
nextlink in the response to fetch subsequent pages.Proposed fix sketch
try: - data = self.client.get( - f"/2.0/repositories/{self._workspace}", - params={"pagelen": 100}, - ) - for r in data.get("values", []): + url = f"/2.0/repositories/{self._workspace}" + params = {"pagelen": 100} + while url: + data = self.client.get(url, params=params) + for r in data.get("values", []): + ... # existing repo mapping + url = data.get("next") + params = {} # next URL already contains params🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/wellcode_cli/integrations/bitbucket/provider.py` around lines 60 - 78, The Bitbucket methods are only fetching the first page; update the repository listing in the method that builds repos (the code using self.client.get("/2.0/repositories/{self._workspace}", params={"pagelen": 100})) to follow pagination by looping: fetch the initial page, process data.get("values", []), then while data.get("next") is present call self.client.get(data["next"]) (no params) and continue appending results until no next link; do the same for get_pull_requests (the function that calls self.client.get(..., params={"pagelen": 50}))—replace the single-call logic with a loop that follows data["next"] and aggregates all pages before returning.src/wellcode_cli/web/static/index.html-313-317 (1)
313-317:⚠️ Potential issue | 🟠 Major
runCollect()issues: impliciteventand GET for a mutating action.
event.targetrelies on the impliciteventglobal, which is non-standard in strict mode. Pass the event explicitly:onclick="runCollect(event)"andfunction runCollect(e) { const btn = e.target; … }.- The snapshot collection is triggered via a plain
GETto/api/v1/metrics/snapshots. If this causes data collection (a side effect), it should be aPOST. GET requests can be inadvertently triggered by prefetchers, link scanners, etc.Proposed fix
- <button onclick="runCollect()" class="bg-indigo-600 ...">Collect</button> + <button onclick="runCollect(event)" class="bg-indigo-600 ...">Collect</button>- async function runCollect() { - const btn = event.target; btn.textContent = 'Collecting...'; btn.disabled = true; - try { await fetch(API+'/metrics/snapshots'); } catch(e) {} + async function runCollect(e) { + const btn = e.target; btn.textContent = 'Collecting...'; btn.disabled = true; + try { await fetch(API+'/metrics/snapshots', { method: 'POST' }); } catch(e) {}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/wellcode_cli/web/static/index.html` around lines 313 - 317, runCollect currently uses the implicit global event and issues a GET for a mutating action; change the function signature to accept an event parameter (e.g., function runCollect(e) { const btn = e.target; ... }) and update the HTML caller to pass the event (onclick="runCollect(event)"), and change the fetch call in runCollect from a GET to a POST to /api/v1/metrics/snapshots (include appropriate headers like 'Content-Type': 'application/json' if needed) and handle errors (log or surface them) so the button state/reset still runs in the finally/timeout path; reference the runCollect function and the fetch call to locate edits.src/wellcode_cli/web/static/index.html-224-227 (1)
224-227:⚠️ Potential issue | 🟠 MajorXSS vulnerability: unsanitized API data rendered via
innerHTML.
c.usernameandc.pr_countare interpolated directly into aninnerHTMLassignment. If a username contains malicious HTML (e.g.,<img onerror=...>), it will execute in the browser. The same issue affects the AI tools table (lines 249–257) and surveys list (lines 279–284).Use
textContentfor plain-text values, or sanitize/escape HTML before inserting. A minimal escape helper:🔒 Proposed fix — add an HTML escape utility and use it
const API = '/api/v1'; let charts = {}; +function esc(s) { const d = document.createElement('div'); d.textContent = s; return d.innerHTML; }Then use
esc()around all interpolated values, e.g.:- `<div class="flex justify-between items-center"><span>${c.username}</span>…` + `<div class="flex justify-between items-center"><span>${esc(c.username)}</span>…`🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/wellcode_cli/web/static/index.html` around lines 224 - 227, Replace unsafe innerHTML interpolation with safe text insertion: stop building HTML strings with untrusted values and instead create DOM nodes and set their textContent (or use a small escape helper) for the elements referenced (the 'top-contributors' container, the AI tools table rendering code, and the surveys list rendering). Locate the code that fills document.getElementById('top-contributors') and the similar blocks that populate the AI tools and surveys UI, and change them to create elements (e.g., div/tr/td/list items), assign user-visible strings via textContent (or pass through an HTML escape function), and then appendChild/join those nodes to the container to eliminate XSS risk.src/wellcode_cli/integrations/bitbucket/provider.py-107-110 (1)
107-110:⚠️ Potential issue | 🟠 MajorTimezone replacement is unconditional — differs from GitHub provider.
Lines 107 and 109 always call
since.replace(tzinfo=timezone.utc)regardless of whethersincealready has a timezone. Ifsincehas a non-UTC timezone, this silently changes it without converting. The GitHub provider checksif since.tzinfo is Nonefirst. Apply the same guard here for consistency and correctness.Proposed fix
- if created_at < since.replace(tzinfo=timezone.utc): + since_utc = since.replace(tzinfo=timezone.utc) if since.tzinfo is None else since + until_utc = until.replace(tzinfo=timezone.utc) if until.tzinfo is None else until + if created_at < since_utc: break - if created_at > until.replace(tzinfo=timezone.utc): + if created_at > until_utc: continueBetter yet, compute
since_utc/until_utconce before the loop (as done in the GitHub provider).🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/wellcode_cli/integrations/bitbucket/provider.py` around lines 107 - 110, The timezone handling for `since`/`until` is incorrect: don’t unconditionally call `since.replace(tzinfo=timezone.utc)`/`until.replace(...)` inside the loop (which can silently change non-UTC offsets); instead compute `since_utc` and `until_utc` once before iterating (as the GitHub provider does) by converting only when needed — e.g., if `since.tzinfo is None` set UTC or otherwise convert to UTC — and then use `since_utc`/`until_utc` in the comparisons with `created_at`; update references in the method in provider.py (the block comparing `created_at < ...` / `created_at > ...`) to use those precomputed UTC variables.src/wellcode_cli/integrations/github/provider.py-180-186 (1)
180-186:⚠️ Potential issue | 🟠 MajorDeployment iteration assumes undocumented ordering —
breakstatement is unsafe.
repo.get_deployments()does not support a sort parameter, and the GitHub REST API does not document any guaranteed ordering for deployments. Thebreakstatement at line 184 assumes deployments are returned in descending order bycreated_at, which is not guaranteed. If deployments are returned in a different order, iteration will stop prematurely and miss deployments within the requested time range.Replace
breakwithcontinueto safely filter all deployments, or add explicit API query parameters if GitHub documents a stable sort order.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/wellcode_cli/integrations/github/provider.py` around lines 180 - 186, The loop over repo.get_deployments() assumes descending created_at order and uses break when created < since_utc, which can prematurely stop iteration; change the logic in the loop that processes deploy.created_at (the variable named created) to avoid breaking out early—replace the break with continue (or otherwise ensure full iteration/filtering) so all deployments are checked against since_utc and until_utc; update the processing in the block around repo.get_deployments(), created, since_utc, and until_utc to use continue instead of break.src/wellcode_cli/db/repository.py-313-338 (1)
313-338: 🛠️ Refactor suggestion | 🟠 Major
MetricStore(and all callers) lacks safe session lifecycle management.All API routes and CLI commands follow the pattern
session = get_session(); store = MetricStore(session); ...; session.close()withouttry/finally. If any code raises between open and close, the session leaks. Consider makingMetricStorea context manager or using FastAPI'sDependswith a generator for session lifecycle.Example: context manager approach
class MetricStore: """Unified access to all metric repositories.""" def __init__(self, session: Session): self.session = session # ... repos ... + def __enter__(self): + return self + + def __exit__(self, exc_type, exc_val, exc_tb): + if exc_type: + self.rollback() + self.close() + return False + def commit(self): self.session.commit() def rollback(self): self.session.rollback() def close(self): self.session.close()🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/wellcode_cli/db/repository.py` around lines 313 - 338, MetricStore currently requires callers to manually close the Session and can leak sessions; implement context-manager lifecycle on MetricStore by adding __enter__ and __exit__ (or async variants if used async) that return self, commit on normal exit, rollback on exception, and always close the session, and update callers to use "with MetricStore(session) as store:" (or change FastAPI usage to a dependency generator that yields MetricStore); keep existing methods commit/rollback/close but ensure __exit__ orchestrates them safely to prevent leaks.src/wellcode_cli/api/routes/metrics.py-82-98 (1)
82-98:⚠️ Potential issue | 🟠 MajorORM objects accessed after
session.close()— fragile pattern.Lines 92–95 iterate over
prs(ORM instances) after the session is closed on Line 82. This works today becauseexpire_on_commit=Falseis set and no commit occurs, but it's fragile — any future change that triggers a session flush or adds a lazy-loaded attribute access here will break at runtime. Movesession.close()after all ORM access, or extract the needed values into plain dicts/lists before closing.Proposed fix: move session.close() to the end
- session.close() - - return PRMetricsResponse( + result = PRMetricsResponse( total_prs=len(prs), merged_prs=len(merged), open_prs=len(open_prs), avg_cycle_time_hours=statistics.mean(cycle_times) if cycle_times else 0, avg_time_to_merge_hours=statistics.mean(merge_times) if merge_times else 0, avg_time_to_first_review_hours=statistics.mean(review_times) if review_times else 0, avg_pr_size=statistics.mean(sizes) if sizes else 0, revert_count=sum(1 for p in prs if p.is_revert), hotfix_count=sum(1 for p in prs if p.is_hotfix), self_merge_count=sum(1 for p in prs if p.is_self_merged), ai_assisted_count=sum(1 for p in prs if p.is_ai_generated), top_contributors=[{"username": u, "pr_count": c} for u, c in top], pr_size_distribution={"small": small, "medium": medium, "large": large}, ) + session.close() + return result🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/wellcode_cli/api/routes/metrics.py` around lines 82 - 98, The code is closing the SQLAlchemy session via session.close() before accessing ORM objects in prs when building the PRMetricsResponse (fields like merged, open_prs, cycle_times, merge_times, review_times, sizes, and counts such as is_revert/is_hotfix/is_self_merged/is_ai_generated); move the session.close() call to after the PRMetricsResponse is fully constructed, or instead materialize all needed values from prs into plain Python lists/dicts (e.g., compute merged, open_prs, cycle_times, merge_times, review_times, sizes, top, small/medium/large counts) before calling session.close() so no ORM attributes are accessed after the session is closed.src/wellcode_cli/services/dora.py-120-134 (1)
120-134:⚠️ Potential issue | 🟠 MajorMerged-PR fallback never triggers when non-production deployments exist.
Line 128 checks
if not deployments, but by this pointdeploymentswas already reassigned to include all environments (Line 121–123) when no production deployments were found. If there are staging-only deployments,deploymentsis truthy, so the merged-PR fallback is skipped — yetsuccessful_deploysmay be empty (no "success"/"active" status), yieldingdeploy_freq = 0.Consider checking
if not successful_deploysinstead:- if not deployments: + if not successful_deploys: merged_prs = store.pull_requests.get_merged_by_period(🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/wellcode_cli/services/dora.py` around lines 120 - 134, The fallback to use merged PRs should run when there are no successful deployments, not when the raw deployments list is empty; change the conditional that triggers the merged-PR fallback from checking deployments to checking successful_deploys (i.e., replace the `if not deployments` branch with `if not successful_deploys`), and ensure you recompute deploy_freq and set successful_deploys from merged_prs (using store.pull_requests.get_merged_by_period and filtering by base_branch) just as before so deploy_freq reflects merged-to-main activity when there are no "success"/"active" deployments.src/wellcode_cli/db/repository.py-39-48 (1)
39-48:⚠️ Potential issue | 🟠 Major
OrganizationRepo.get_or_createlooks up bynameonly — should includeprovider.This matches the unique constraint issue flagged in
models.py. Once the constraint is changed to(provider, name), this query must also filter onproviderto avoid returning the wrong org.class OrganizationRepo(BaseRepository): def get_or_create(self, name: str, provider: str, **kwargs) -> Organization: org = self.session.execute( - select(Organization).where(Organization.name == name) + select(Organization).where( + Organization.name == name, + Organization.provider == provider, + ) ).scalar_one_or_none()🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/wellcode_cli/db/repository.py` around lines 39 - 48, The get_or_create implementation in OrganizationRepo currently queries only by name; update the query in OrganizationRepo.get_or_create to filter by both Organization.name == name and Organization.provider == provider so it returns or creates the correct organization under the (provider, name) uniqueness rule; keep the creation path as-is (org = Organization(name=name, provider=provider, **kwargs), add, flush) but ensure the select uses both identifying columns in the WHERE clause.src/wellcode_cli/api/routes/metrics.py-50-57 (1)
50-57:⚠️ Potential issue | 🟠 MajorMalformed date strings will cause an unhandled 500 error.
datetime.fromisoformat(end)anddatetime.fromisoformat(start)on Lines 51–52 raiseValueErroron invalid input, producing an opaque 500 instead of a clear 422. Wrap in a try/except or use a Pydantic type to validate dates.Proposed fix
+from fastapi import APIRouter, Depends, HTTPException, Query ... + try: + end_dt = datetime.fromisoformat(end) if end else now + start_dt = datetime.fromisoformat(start) if start else end_dt - timedelta(days=30) + except ValueError: + raise HTTPException(status_code=422, detail="Invalid date format. Use YYYY-MM-DD.")🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/wellcode_cli/api/routes/metrics.py` around lines 50 - 57, The code currently calls datetime.fromisoformat(end) and datetime.fromisoformat(start) directly (producing start_dt/end_dt) which will raise ValueError on malformed dates; modify the metrics route to validate parsing by wrapping those calls in a try/except that catches ValueError (or use pydantic parsing) and returns a clear 422 response (raise FastAPI's HTTPException(status_code=422, detail="invalid date format for 'start'/'end'")) instead of letting the exception bubble; ensure you update the logic around start_dt/end_dt and still pass validated datetimes to MetricStore.pull_requests.get_by_period so consumers receive a proper validation error rather than a 500.src/wellcode_cli/db/models.py-31-45 (1)
31-45:⚠️ Potential issue | 🟠 Major
Organization.nameunique constraint will collide across providers.If the same organization name exists on multiple SCM providers (e.g., "acme" on both GitHub and GitLab), the
unique=Trueonnamealone will reject the second insert. This should be a compound unique constraint on(provider, name), consistent with howRepositoryandDeveloperare modeled.Proposed fix
class Organization(Base): __tablename__ = "organizations" id = Column(Integer, primary_key=True, autoincrement=True) - name = Column(String(255), nullable=False, unique=True) + name = Column(String(255), nullable=False) provider = Column(String(50), nullable=False) # github, gitlab, bitbucket external_id = Column(String(255)) avatar_url = Column(Text) created_at = Column(DateTime, default=lambda: datetime.now(timezone.utc)) updated_at = Column(DateTime, default=lambda: datetime.now(timezone.utc), onupdate=lambda: datetime.now(timezone.utc)) repositories = relationship("Repository", back_populates="organization") teams = relationship("Team", back_populates="organization") + + __table_args__ = ( + UniqueConstraint("provider", "name", name="uq_org_provider_name"), + )🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/wellcode_cli/db/models.py` around lines 31 - 45, The Organization model currently sets unique=True on the name column which will conflict across different providers; remove unique=True from Organization.name and add a composite unique constraint on (provider, name) in the Organization class (e.g., via __table_args__ = (UniqueConstraint("provider", "name", name="uq_organization_provider_name"),) ) so uniqueness matches Repository/Developer modeling; update any migrations to reflect removing the single-column unique index and creating the new composite unique constraint.src/wellcode_cli/services/ai_metrics.py-113-115 (1)
113-115:⚠️ Potential issue | 🟠 MajorBare
except Exceptionswallows all errors from the HTTP call.This catches and logs everything (including unexpected issues like
KeyboardInterruptvia the outerexcept Exception). More importantly, if the request partially committed data before the exception (e.g., some days were added viastore.ai_usage.addon Line 103 before a failure in a later iteration), the caller won't know the store is in a dirty state.Consider narrowing to
requests.RequestExceptionand rolling back on failure.Proposed fix
- except Exception as e: - logger.error("Error fetching Copilot metrics: %s", e) + except requests.RequestException as e: + logger.error("Error fetching Copilot metrics: %s", e) + store.rollback()🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/wellcode_cli/services/ai_metrics.py` around lines 113 - 115, Narrow the broad except in the Copilot metrics fetch block to only catch network/HTTP errors (e.g., requests.RequestException) instead of Exception, and on such a failure undo the partial updates made by store.ai_usage.add (or call the store's rollback/cleanup method) before re-raising or returning an error; update the except block that currently logs via logger.error("Error fetching Copilot metrics: %s", e) to perform rollback of the partial state from store.ai_usage.add and then log the RequestException and propagate it so callers know the store may have been modified.src/wellcode_cli/api/app.py-28-34 (1)
28-34:⚠️ Potential issue | 🟠 Major
allow_origins=["*"]combined withallow_credentials=Trueis a security misconfiguration.Per the CORS specification, browsers reject responses with
Access-Control-Allow-Origin: *when credentials (cookies, auth headers) are included. FastAPI'sCORSMiddlewareworks around this by reflecting the request'sOriginheader whenallow_credentials=Trueandallow_origins=["*"], effectively allowing any origin with credentials — which defeats the purpose of CORS protection.For a development/internal tool this may be acceptable, but for production deployments restrict
allow_originsto known dashboard origins.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/wellcode_cli/api/app.py` around lines 28 - 34, The CORS middleware configuration in app.add_middleware using CORSMiddleware with allow_origins=["*"] and allow_credentials=True is insecure; change it to use a restricted list of allowed origins (replace ["*"] with a list of trusted origins) and keep allow_credentials=True only when those specific origins are supplied, or alternatively set allow_credentials=False for public origins; update the app.add_middleware CORSMiddleware call to reference the concrete allowed origin strings instead of "*" (and document/update any env/config variable that supplies those origins).src/wellcode_cli/api/routes/ai_metrics.py-58-61 (1)
58-61:⚠️ Potential issue | 🟠 MajorDB session leak on exception path.
If
compute_ai_impact(Line 60) orstore.ai_usage.get_by_period(Line 100) raises,session.close()is never reached. Usetry/finallyor, better, a FastAPI dependency withyieldto guarantee cleanup.♻️ Suggested fix using try/finally (minimal change)
session = get_session() - store = MetricStore(session) - impact = compute_ai_impact(store, start_dt, end_dt) - session.close() + try: + store = MetricStore(session) + impact = compute_ai_impact(store, start_dt, end_dt) + finally: + session.close()Apply the same pattern to
get_ai_usage.Also applies to: 98-101
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/wellcode_cli/api/routes/ai_metrics.py` around lines 58 - 61, The DB session is leaked if compute_ai_impact or store.ai_usage.get_by_period raises because session.close() is called unconditionally after the call; change both call sites to ensure session is always closed (use try/finally around session usage or convert get_session to a FastAPI dependency with yield). Specifically, wrap the creation of session and MetricStore and the call to compute_ai_impact (and likewise the get_ai_usage flow that calls store.ai_usage.get_by_period) in a try/finally that calls session.close() in finally, or refactor to a dependency that yields the session so cleanup is guaranteed.src/wellcode_cli/jira/jira_display.py-155-155 (1)
155-155:⚠️ Potential issue | 🟠 MajorIncorrect unit label: variance is a percentage, not hours.
avg_varianceinJiraEstimationMetricsis computed as((actual_hours - expected_hours) / expected_hours) * 100— a percentage. The display label on Line 155 says"{avg_variance:.1f} hours"but should say"%"or"percent".🐛 Proposed fix
- + f"[bold]Average Variance:[/] {avg_variance:.1f} hours", + + f"[bold]Average Variance:[/] {avg_variance:.1f}%",🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/wellcode_cli/jira/jira_display.py` at line 155, The display string wrongly labels avg_variance as hours; update the formatted output in jira_display.py (the line that builds the string "[bold]Average Variance:[/] {avg_variance:.1f} hours") to use a percent unit instead (e.g., "{avg_variance:.1f}%" or " {avg_variance:.1f} percent") so the JiraEstimationMetrics avg_variance value is shown with the correct percent unit.src/wellcode_cli/services/collector.py-40-61 (1)
40-61:⚠️ Potential issue | 🟠 MajorSession not cleaned up if
store.snapshots.create()raises.If
store.snapshots.create(period_start, period_end)on Line 51 throws an exception, the session obtained on Line 47 is never closed. The early-return path for no providers (Lines 56-61) correctly closes, but an exception before that point leaks the session.♻️ Wrap in try/finally
def collect_all(...) -> dict: init_db() session = get_session() - store = MetricStore(session) - - start_time = time.time() - snapshot = store.snapshots.create(period_start, period_end) + try: + store = MetricStore(session) + start_time = time.time() + snapshot = store.snapshots.create(period_start, period_end) + ... # rest of function body + finally: + store.close()🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/wellcode_cli/services/collector.py` around lines 40 - 61, collect_all may leak the DB session if store.snapshots.create(...) raises; wrap the lifetime of the MetricStore/session so it is always closed: in the collect_all function (around init_db(), get_session(), MetricStore(...)) ensure you use a try/finally (or context-manager-like pattern) so that store.close() is called in the finally block if any exception occurs during snapshot creation or later processing; reference the MetricStore instance (store), the call store.snapshots.create, and ensure you still perform store.commit() and store.snapshots.fail(...) where appropriate before closing.src/wellcode_cli/jira/jira_metrics.py-265-293 (1)
265-293:⚠️ Potential issue | 🟠 Major
calculate_work_hoursdoesn't account for partial first/last days.The function always computes work hours as
day_end - day_startwhereday_startis forced to 9:00 AM. Ifstart_dateis 3:00 PM on a weekday, it still counts 8 hours (9 AM–5 PM) for that day instead of the correct 2 hours (3 PM–5 PM). Similarly, the last day ignores the actualend_datetime when it falls between 9 AM and 5 PM (it's handled bymin(day_end, end_date)butday_startis not adjusted tomax(day_start, current_date)).🐛 Proposed fix
while current_date < end_date: if current_date.weekday() < 5: day_end = min( current_date.replace(hour=17, minute=0, second=0, microsecond=0), end_date, ) - day_start = current_date.replace(hour=9, minute=0, second=0, microsecond=0) + day_start = max( + current_date.replace(hour=9, minute=0, second=0, microsecond=0), + current_date, + ) if day_end > day_start: work_hours = (day_end - day_start).total_seconds() / 3600 total_hours += min(8, work_hours)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/wellcode_cli/jira/jira_metrics.py` around lines 265 - 293, The function calculate_work_hours miscounts partial first/last days by always using day_start = 9:00; fix by computing day_start = max(current_date, current_date.replace(hour=9,...)) and day_end = min(end_date, current_date.replace(hour=17,...)) so the first-day start time respects start_date and the last-day end time respects end_date; keep the existing weekday check and the min(8, work_hours) guard but ensure day_end > day_start before adding hours and that current_date advances by one day at 09:00 as before.src/wellcode_cli/services/collector.py-146-147 (1)
146-147:⚠️ Potential issue | 🟠 Major
cycle_time_hoursshould usetime_to_merge_hoursinstead oflead_time.Line 147 incorrectly assigns
cycle_time_hours=lead_time, making it identical tolead_time_hours=lead_timeon line 146. These are distinct metrics: lead time measures first commit → merge, while cycle time should measure creation → merge. The variablettm(time_to_merge_hours), which calculates (merged_at - created_at) / 3600, is the appropriate value forcycle_time_hours.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/wellcode_cli/services/collector.py` around lines 146 - 147, The assignment mistakenly sets cycle_time_hours to lead_time; change the value passed to the metric so cycle_time_hours uses the time_to_merge_hours variable (ttm) instead of lead_time. Locate where lead_time_hours=lead_time and cycle_time_hours=lead_time are set (look for the lead_time_hours and cycle_time_hours kwargs in the same call) and replace cycle_time_hours=lead_time with cycle_time_hours=time_to_merge_hours (or cycle_time_hours=ttm) so cycle time reflects (merged_at - created_at)/3600 rather than the first-commit lead time.src/wellcode_cli/jira/models/metrics.py-156-163 (1)
156-163:⚠️ Potential issue | 🟠 Major
statistics.quantilesrequires at least 2 data points.
statistics.quantiles(self.cycle_times, n=20)raisesStatisticsErroriflen(self.cycle_times) < 2. The guardif self.cycle_timesonly ensures the list is non-empty (i.e., could have exactly 1 element).🐛 Proposed fix
"cycle_time_p95": ( - statistics.quantiles(self.cycle_times, n=20)[-1] - if self.cycle_times + statistics.quantiles(self.cycle_times, n=20)[-1] + if len(self.cycle_times) >= 2 else 0 ),Python statistics.quantiles minimum data points requirement🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/wellcode_cli/jira/models/metrics.py` around lines 156 - 163, The p95 calculation calls statistics.quantiles(self.cycle_times, n=20) which raises StatisticsError when there is only one data point; update the guard in the "cycle_time_p95" expression to use quantiles only if len(self.cycle_times) >= 2 and otherwise fall back to a safe value (e.g., statistics.median(self.cycle_times) when there's one item, or 0 when empty). Modify the check around self.cycle_times for "cycle_time_p95" to reference len(self.cycle_times) >= 2 and use self.cycle_times[0] or statistics.median(self.cycle_times) as the single-point fallback so statistics.quantiles is never called with fewer than 2 elements.src/wellcode_cli/main.py-146-149 (1)
146-149:⚠️ Potential issue | 🟠 MajorSession resource leak in
doraandai_metrics_cmdcommandsIn both
dora(lines 146–149) andai_metrics_cmd(lines 211–214),session.close()is called unconditionally after the computation call. Ifcompute_doraorcompute_ai_impactraises an exception,session.close()is never reached, leaking the connection.🔧 Fix with try/finally (dora and ai_metrics_cmd)
session = get_session() - store = MetricStore(session) - metrics = compute_dora(store, start_date, end_date, repo_id=repo_id, team_id=team_id) - session.close() + try: + store = MetricStore(session) + metrics = compute_dora(store, start_date, end_date, repo_id=repo_id, team_id=team_id) + finally: + session.close()Apply the same pattern in
ai_metrics_cmdaroundcompute_ai_impact.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/wellcode_cli/main.py` around lines 146 - 149, The session created by get_session() is closed unconditionally after calling compute_dora/compute_ai_impact which can raise and prevent session.close() from running; wrap the usage of session/MetricStore and the compute_* call in a try/finally so session.close() always executes (e.g., in dora where session = get_session(); store = MetricStore(session); metrics = compute_dora(...), and likewise in ai_metrics_cmd around compute_ai_impact), ensuring any exceptions propagate but the session is closed in the finally block.src/wellcode_cli/main.py-98-101 (1)
98-101:⚠️ Potential issue | 🟠 MajorTimezone mismatch:
click.DateTime()returns naive datetimes but fallbacks produce timezone-aware datetimes
click.DateTime()without aformatsoverride produces naivedatetimeobjects. The fallback path setsend_date = datetime.now(timezone.utc)which is aware. If a user passes--start-date 2024-01-01without--end-date,start_dateis naive andend_dateis aware; any subsequent subtraction (e.g.,period_end - period_startinsidecollect_all) raisesTypeError: can't subtract offset-naive and offset-aware datetimes.The same pattern exists in the
doracommand (lines 141–144) andai_metrics_cmdcommand (lines 206–209).🔧 Proposed fix (normalize click inputs to UTC)
- if end_date is None: - end_date = datetime.now(timezone.utc) - if start_date is None: - start_date = end_date - timedelta(days=days) + if end_date is None: + end_date = datetime.now(timezone.utc) + else: + end_date = end_date.replace(tzinfo=timezone.utc) + if start_date is None: + start_date = end_date - timedelta(days=days) + else: + start_date = start_date.replace(tzinfo=timezone.utc)Apply the same fix in
dora(lines 141–144) andai_metrics_cmd(lines 206–209).🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/wellcode_cli/main.py` around lines 98 - 101, Normalize click DateTime inputs to UTC by making naive datetimes timezone-aware before doing arithmetic: in the command that sets end_date/start_date (the block handling variables start_date and end_date) ensure that if end_date is None you keep datetime.now(timezone.utc), and if either start_date or end_date is provided but tzinfo is None, attach timezone.utc (or convert to UTC) so both become aware datetimes; apply the same fix in the dora command and the ai_metrics_cmd command where start_date/end_date are computed so subsequent operations (e.g., collect_all period_end - period_start) do not mix naive and aware datetimes.src/wellcode_cli/api/routes/surveys.py-88-104 (1)
88-104:⚠️ Potential issue | 🟠 MajorSession resource leak in
list_active_surveys
session.close()on line 103 is not guarded bytry/finally. An exception duringget_active_surveys()or the result iteration leaks the connection.🔧 Fix
def list_active_surveys(): session = get_session() - store = MetricStore(session) - surveys = store.surveys.get_active_surveys() - result = [] - for s in surveys: - result.append(SurveyResponseModel(...)) - session.close() - return result + try: + store = MetricStore(session) + surveys = store.surveys.get_active_surveys() + return [SurveyResponseModel(...) for s in surveys] + finally: + session.close()Consider adopting a FastAPI dependency-injection pattern (
Depends(get_db)) for all endpoints to handle session lifecycle uniformly and avoid repeating this pattern.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/wellcode_cli/api/routes/surveys.py` around lines 88 - 104, The list_active_surveys function opens a DB session via get_session() and can leak it if get_active_surveys() or the iteration raises; update list_active_surveys to ensure the session is always closed by either (preferred) switching the endpoint to use FastAPI dependency injection with a get_db/Depends parameter and passing that session into MetricStore, or (alternatively) wrapping the session usage in a try/finally where session.close() is called in finally; make changes around the symbols list_active_surveys, get_session, MetricStore and ensure the returned SurveyResponseModel mapping remains the same.src/wellcode_cli/main.py-68-83 (1)
68-83:⚠️ Potential issue | 🟠 MajorScheduler is not shut down if
uvicorn.run()raises, and--reload+--scheduleare incompatibleTwo issues:
start_scheduler()is called unconditionally before the blockinguvicorn.run(). If uvicorn fails to bind (e.g., port already in use), the scheduler is left running in the background with no way to stop it. Wrap the uvicorn call in atry/finallyand call_scheduler.shutdown()on failure.When
--reloadis active, uvicorn starts a reloader parent process and one or more worker child processes. The scheduler is launched in the parent (reloader) process, which does not handle HTTP requests. Worker processes are separate and unaware of the scheduler. Combining--reloadand--scheduleis almost certainly unintentional during development (reload implies development) and should either be explicitly disallowed or at minimum documented.🔧 Suggested guard
if schedule: from .workers.scheduler import start_scheduler - start_scheduler(interval_hours=interval) - console.print(f"[green]Background collection enabled (every {interval}h)[/]") + if reload: + console.print("[yellow]Warning: --reload disables background scheduling (use in dev only)[/]") + else: + start_scheduler(interval_hours=interval) + console.print(f"[green]Background collection enabled (every {interval}h)[/]") console.print(f"\n[bold blue]Wellcode[/] v{__version__}") console.print(f"[green]API server starting at http://{host}:{port}[/]") console.print(f"[dim]API docs at http://{host}:{port}/docs[/]\n") +scheduler = None +try: uvicorn.run( "wellcode_cli.api.app:app", host=host, port=port, reload=reload, log_level="info", ) +finally: + if schedule and not reload and scheduler: + scheduler.shutdown()🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/wellcode_cli/main.py` around lines 68 - 83, Wrap the uvicorn.run call in a try/finally so any background scheduler started by start_scheduler(interval_hours=interval) is reliably stopped on error by calling the scheduler shutdown method (call _scheduler.shutdown() or the appropriate shutdown API exposed by your scheduler) in the finally block; additionally, prevent the incompatible combination of --schedule and --reload by checking the reload flag when schedule is requested and aborting (or printing a clear error and exiting) if reload is true, or alternatively require the user to pass an explicit --allow-schedule-with-reload flag before starting the scheduler, and surface a clear console message referencing start_scheduler, _scheduler.shutdown, schedule and reload so maintainers can locate the change.src/wellcode_cli/api/routes/dora.py-43-60 (1)
43-60:⚠️ Potential issue | 🟠 MajorThree issues in
get_dora_metrics: naive/aware mismatch, unhandledValueError, and session leak
Timezone mismatch: When only
startis provided,end_dt = datetime.now(timezone.utc)is tz-aware whilestart_dt = datetime.fromisoformat(start)is tz-naive (e.g.,"2024-01-01").compute_dorasubtracts them, raisingTypeError. The fix is to normalize parsed datetimes to UTC.Unhandled
ValueError: An invalidstart/endstring (e.g.,"not-a-date") causesdatetime.fromisoformatto raiseValueError, resulting in a 500 instead of 422. Wrap parsing in a try/except and raiseHTTPException(422, ...).Session leak:
session.close()on line 52 is outside atry/finallyblock; an exception incompute_doraleaks the connection.🔧 Proposed fix
- now = datetime.now(timezone.utc) - end_dt = datetime.fromisoformat(end) if end else now - start_dt = datetime.fromisoformat(start) if start else end_dt - timedelta(days=30) - - session = get_session() - store = MetricStore(session) - - metrics = compute_dora(store, start_dt, end_dt, repo_id=repo_id, team_id=team_id) - - session.close() + from fastapi import HTTPException + now = datetime.now(timezone.utc) + try: + end_dt = datetime.fromisoformat(end).replace(tzinfo=timezone.utc) if end else now + start_dt = datetime.fromisoformat(start).replace(tzinfo=timezone.utc) if start else end_dt - timedelta(days=30) + except ValueError as exc: + raise HTTPException(status_code=422, detail=f"Invalid date format: {exc}") from exc + + session = get_session() + try: + store = MetricStore(session) + metrics = compute_dora(store, start_dt, end_dt, repo_id=repo_id, team_id=team_id) + finally: + session.close()🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/wellcode_cli/api/routes/dora.py` around lines 43 - 60, The get_dora_metrics flow has three issues: naive/aware datetime mismatch, unhandled parse errors, and a session leak; fix by (1) wrapping parsing of start and end (datetime.fromisoformat) in try/except that catches ValueError and raises HTTPException(status_code=422, detail="invalid date") so bad inputs return 422, (2) normalize parsed datetimes to UTC (if parsed datetime.tzinfo is None, call datetime.replace(tzinfo=timezone.utc) or convert with astimezone(timezone.utc)) so start_dt and end_dt are timezone-aware and comparable to datetime.now(timezone.utc), and (3) ensure the DB session from get_session() is always closed by using try/finally (or context manager) around compute_dora/store usage (ensure session.close() is in finally) so exceptions do not leak connections; reference functions/objects: datetime.fromisoformat, datetime.now(timezone.utc), compute_dora, get_session, session.close, and HTTPException.src/wellcode_cli/api/routes/dora.py-68-83 (1)
68-83:⚠️ Potential issue | 🟠 MajorSession resource leak in
get_dora_historySame pattern:
session.close()on line 83 is not guarded bytry/finally. An exception instore.dora.get_history()leaks the DB connection.🔧 Fix
session = get_session() - store = MetricStore(session) - items = store.dora.get_history(limit=limit, team_id=team_id) - result = [...] - session.close() - return result + try: + store = MetricStore(session) + items = store.dora.get_history(limit=limit, team_id=team_id) + return [ + DORAHistoryItem(...) + for d in items + ] + finally: + session.close()🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/wellcode_cli/api/routes/dora.py` around lines 68 - 83, The DB session created by get_session() in get_dora_history (where MetricStore(session) and store.dora.get_history(...) are called) must be closed in a finally block to avoid leaking connections if store.dora.get_history raises; wrap the creation/use of session, MetricStore, and the list comprehension in a try/finally (or convert get_session to a context manager) and move session.close() into the finally so the session is always closed even on exceptions.src/wellcode_cli/api/routes/surveys.py-107-110 (1)
107-110:⚠️ Potential issue | 🟠 Major
submit_survey_responsereturns 500 for non-existentsurvey_id
submit_responseinternally callsstore.surveys.add_response()→session.flush(), which triggers anIntegrityErrorifsurvey_idhas no matching row. This propagates as an unhandled 500. Catch the error and return a 404 or 422.🔧 Proposed fix
+from sqlalchemy.exc import IntegrityError + `@router.post`("/respond") def submit_survey_response(req: SubmitResponseRequest): - response = submit_response(req.survey_id, req.answers, req.developer_id) - return {"id": response.id, "submitted_at": response.submitted_at.isoformat()} + try: + response = submit_response(req.survey_id, req.answers, req.developer_id) + except IntegrityError: + raise HTTPException(status_code=404, detail=f"Survey {req.survey_id} not found") + return {"id": response.id, "submitted_at": response.submitted_at.isoformat()}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/wellcode_cli/api/routes/surveys.py` around lines 107 - 110, submit_survey_response currently lets an IntegrityError from submit_response → store.surveys.add_response → session.flush bubble up and return a 500 for non-existent survey_id; wrap the call to submit_response in a try/except that catches sqlalchemy.exc.IntegrityError (import it) and raise fastapi.HTTPException with the appropriate status (e.g., 404 Not Found or 422 Unprocessable Entity) and a clear message like "survey not found" or "invalid survey_id"; locate submit_survey_response and modify it to handle IntegrityError, preserving successful behavior (return id and submitted_at) otherwise.src/wellcode_cli/services/surveys.py-62-178 (1)
62-178:⚠️ Potential issue | 🟠 MajorSession resource leaks in all three service functions
create_survey_from_template,submit_response, andanalyze_surveyall callsession.close()as the last statement, with notry/finallyguard. Any exception in the function body (DB error, stats calculation, etc.) leaves the connection unclosed. The fix is consistent across all three:🔧 Pattern to apply to all three functions
def create_survey_from_template(...) -> Survey: session = get_session() - store = MetricStore(session) - ... - store.commit() - session.close() - return survey + try: + store = MetricStore(session) + ... + store.commit() + return survey + finally: + session.close()Apply the same
try/finallywrapping insubmit_response(lines 103–118) andanalyze_survey(lines 124–178). Note thatanalyze_surveyalready has an early return on line 141–142; withtry/finally, the early-return path is also covered correctly.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/wellcode_cli/services/surveys.py` around lines 62 - 178, All three functions (create_survey_from_template, submit_response, analyze_survey) leak DB sessions on exceptions; wrap the body that uses get_session() and MetricStore(session) in a try/finally and call session.close() in the finally block so the session is always closed, and ensure store.commit() stays inside the try (or is only called on success); reference the existing get_session(), MetricStore, store.commit(), and store.surveys.* calls to locate where to wrap and keep early returns (like the no-responses path in analyze_survey) inside the try so finally always runs.
| healthcheck: | ||
| test: ["CMD-LINE", "pg_isready -U wellcode"] | ||
| interval: 5s | ||
| timeout: 5s | ||
| retries: 5 |
There was a problem hiding this comment.
CMD-LINE is not a valid Docker healthcheck format — the postgres container will never report healthy, blocking the wellcode service from starting.
Docker healthcheck test accepts CMD (exec form) or CMD-SHELL (string passed to /bin/sh -c). CMD-LINE is unrecognized and will cause the check to always fail. Since wellcode depends on condition: service_healthy, it will never start.
🐛 Proposed fix
healthcheck:
- test: ["CMD-LINE", "pg_isready -U wellcode"]
+ test: ["CMD-SHELL", "pg_isready -U wellcode"]
interval: 5s
timeout: 5s
retries: 5📝 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.
| healthcheck: | |
| test: ["CMD-LINE", "pg_isready -U wellcode"] | |
| interval: 5s | |
| timeout: 5s | |
| retries: 5 | |
| healthcheck: | |
| test: ["CMD-SHELL", "pg_isready -U wellcode"] | |
| interval: 5s | |
| timeout: 5s | |
| retries: 5 |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@docker-compose.yml` around lines 37 - 41, The healthcheck uses an invalid
test keyword "CMD-LINE" so the postgres service never becomes healthy; replace
the test entry in the healthcheck for the postgres service to a valid form
(either exec form using "CMD" with separate args or shell form using "CMD-SHELL"
with a single string) so pg_isready runs correctly (e.g. use the "healthcheck"
block's test with "CMD" and args referencing pg_isready and "-U wellcode" or
"CMD-SHELL" with "pg_isready -U wellcode"); also ensure the dependent service
that uses condition: service_healthy (wellcode) can now start when the
healthcheck passes.
| for pr in repo.get_pulls(state="all", sort="updated", direction="desc"): | ||
| created = pr.created_at.replace(tzinfo=timezone.utc) if pr.created_at.tzinfo is None else pr.created_at | ||
| if created < since_utc: | ||
| break | ||
| if created > until_utc: | ||
| continue |
There was a problem hiding this comment.
Critical: break on created < since is incorrect when sorting by updated.
PRs are fetched sorted by updated (descending), but the loop breaks when created < since_utc. A PR created months ago but updated yesterday will appear first and immediately terminate the loop, causing all subsequent PRs (including those created within the window) to be skipped.
Either sort by created or replace the break with continue:
Proposed fix — sort by created
- for pr in repo.get_pulls(state="all", sort="updated", direction="desc"):
+ for pr in repo.get_pulls(state="all", sort="created", direction="desc"):📝 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.
| for pr in repo.get_pulls(state="all", sort="updated", direction="desc"): | |
| created = pr.created_at.replace(tzinfo=timezone.utc) if pr.created_at.tzinfo is None else pr.created_at | |
| if created < since_utc: | |
| break | |
| if created > until_utc: | |
| continue | |
| for pr in repo.get_pulls(state="all", sort="created", direction="desc"): | |
| created = pr.created_at.replace(tzinfo=timezone.utc) if pr.created_at.tzinfo is None else pr.created_at | |
| if created < since_utc: | |
| break | |
| if created > until_utc: | |
| continue |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/wellcode_cli/integrations/github/provider.py` around lines 80 - 85, The
loop over repo.get_pulls is incorrectly breaking based on created < since_utc
while pulls are sorted by "updated", which can prematurely stop processing;
update the call to repo.get_pulls to sort by created (change sort="updated" to
sort="created") so the list is monotonic by created time, or alternatively keep
sort="updated" and replace the break on created < since_utc with continue to
skip that PR without terminating the loop; adjust the block around
repo.get_pulls, the created variable, and the created < since_utc check
accordingly.
There was a problem hiding this comment.
Pull request overview
This PR turns Wellcode from a CLI-only tool into a deployable metrics platform by adding a persistent database layer, a FastAPI server + static dashboard, and multiple new metric domains (DORA, AI usage/impact, DX surveys, JIRA), along with Docker/Alembic support.
Changes:
- Added a FastAPI app serving REST endpoints and a single-page dashboard UI.
- Introduced a SQLAlchemy persistence layer (models + repositories) and background collection scheduler.
- Added/expanded integrations and metrics: unified SCM providers (GitHub/GitLab/Bitbucket), DORA calculations, AI metrics, DX surveys, and JIRA metrics.
Reviewed changes
Copilot reviewed 36 out of 55 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
src/wellcode_cli/workers/scheduler.py |
Adds APScheduler-based periodic metric collection. |
src/wellcode_cli/web/static/index.html |
New static dashboard UI (Tailwind + Chart.js) consuming the API. |
src/wellcode_cli/services/surveys.py |
DX survey templates, persistence, response submission, analytics. |
src/wellcode_cli/services/dora.py |
DORA metric computation + snapshot persistence. |
src/wellcode_cli/services/collector.py |
Orchestrates collection from configured SCM providers into the DB. |
src/wellcode_cli/services/ai_metrics.py |
Copilot usage collection + AI impact computation + PR AI-tool detection. |
src/wellcode_cli/main.py |
Adds new CLI commands (serve, collect, dora, ai-metrics, survey). |
src/wellcode_cli/jira/models/metrics.py |
Adds JIRA metric dataclasses/aggregation logic. |
src/wellcode_cli/jira/jira_metrics.py |
JIRA API collection + issue/sprint processing. |
src/wellcode_cli/jira/jira_display.py |
Rich UI presentation for JIRA metrics. |
src/wellcode_cli/integrations/scm_protocol.py |
Defines unified SCMProvider protocol + shared SCM dataclasses. |
src/wellcode_cli/integrations/gitlab/provider.py |
Implements SCM provider for GitLab. |
src/wellcode_cli/integrations/github/provider.py |
Implements SCM provider for GitHub. |
src/wellcode_cli/integrations/bitbucket/provider.py |
Implements SCM provider for Bitbucket. |
src/wellcode_cli/github/github_format_ai.py |
Includes JIRA metrics in AI summary payload. |
src/wellcode_cli/db/repository.py |
Adds MetricStore repositories for querying/persisting metrics. |
src/wellcode_cli/db/models.py |
Adds SQLAlchemy models for snapshots, PRs, deployments, AI usage, surveys, DORA, etc. |
src/wellcode_cli/db/migrations/script.py.mako |
Alembic migration template. |
src/wellcode_cli/db/migrations/env.py |
Alembic env configuration wired to Wellcode DB URL. |
src/wellcode_cli/db/engine.py |
DB engine/session factory for SQLite/Postgres (+ SQLite WAL config). |
src/wellcode_cli/db/__init__.py |
Exposes DB helpers. |
src/wellcode_cli/config.py |
Adds config getters for JIRA/GitLab/Bitbucket/DB URL. |
src/wellcode_cli/commands/review.py |
Adds JIRA metrics to review output flow. |
src/wellcode_cli/commands/config.py |
Adds interactive JIRA configuration. |
src/wellcode_cli/api/routes/surveys.py |
Survey endpoints: templates/create/active/respond/analytics. |
src/wellcode_cli/api/routes/metrics.py |
PR metrics endpoint + snapshot listing endpoint. |
src/wellcode_cli/api/routes/health.py |
Health endpoint checks DB connectivity. |
src/wellcode_cli/api/routes/dora.py |
DORA metrics endpoint + DORA history endpoint. |
src/wellcode_cli/api/routes/ai_metrics.py |
AI impact endpoint + AI usage listing endpoint. |
src/wellcode_cli/api/app.py |
FastAPI app wiring: routers, CORS, static mount. |
src/wellcode_cli/__init__.py |
Version bump to 0.2.0. |
requirements.txt |
Adds jira dependency (in addition to pyproject deps). |
pyproject.toml |
Major dependency/config updates; adds wellcode script; Python >=3.10. |
docker-compose.yml |
Adds Docker Compose stack with app + Postgres. |
alembic.ini |
Adds Alembic configuration. |
README.md |
Rewrites docs for new platform features, API, and deployment. |
Dockerfile |
Adds container build for serving the API/dashboard. |
Comments suppressed due to low confidence (8)
src/wellcode_cli/api/app.py:31
- CORS is configured with
allow_origins=['*']while also settingallow_credentials=True. Browsers will reject credentialed requests with a wildcard origin, and it also broadens access unnecessarily. Either setallow_credentials=Falsewhen using*, or restrictallow_originsto an explicit list of trusted origins (recommended).
app.add_middleware(
CORSMiddleware,
allow_origins=["*"],
allow_credentials=True,
allow_methods=["*"],
src/wellcode_cli/services/dora.py:159
- The change failure rate denominator uses
len(successful_deploys)when deployment data exists, which can inflate CFR (failures counted against only successful deploys) and can exceed 1. CFR is typically based on total deployments/changes; consider using total deployments (len(deployments)) when available and only falling back to PR-based proxies when no deployment data exists.
src/wellcode_cli/services/ai_metrics.py:123 AI_COMMIT_PATTERNSis declared but not referenced anywhere in the module/repo, so it’s currently dead code. Either remove it until commit-level AI detection is implemented, or wire it into detection logic to keep behavior and patterns in sync.
src/wellcode_cli/integrations/github/provider.py:113min(r.submitted_at for r in reviews if r.submitted_at)will raiseValueErrorwhenreviewsis non-empty but allsubmitted_atvalues are falsy/missing. Filter into a list first (or guard on the filtered iterable) before callingmin.
src/wellcode_cli/main.py:72- With
--reload, Uvicorn uses a reloader process model; starting APScheduler beforeuvicorn.run()can result in the scheduler running in the parent and/or being started multiple times across reloads. Consider disabling scheduling when reload is enabled, or moving scheduler start/stop into the FastAPI lifespan (startup/shutdown) and ensuring it runs only in the serving process.
src/wellcode_cli/services/dora.py:210 metrics.change_failure_rateis capped withmin(cfr, 1.0), but the persistedDORASnapshot.change_failure_ratestores the uncappedcfr. This makes stored history disagree with API responses and can yield values > 1. Persist the same (capped) value that the service returns, or handle capping consistently at read time.
src/wellcode_cli/services/ai_metrics.py:132AI_BOT_AUTHORSis declared but never used in this module/repo. If bot/AI author filtering isn’t implemented yet, consider removing it for now or integrating it into AI detection so it doesn’t drift out of date.
src/wellcode_cli/workers/scheduler.py:25- Catching
Exceptionhere and logging vialogger.error(..., e)drops the stack trace, which makes scheduled failures hard to diagnose in production. Uselogger.exception("Scheduled collection failed")(or includeexc_info=True) to preserve traceback details.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| const tc = document.getElementById('top-contributors'); | ||
| tc.innerHTML = (prData.top_contributors||[]).map(c => | ||
| `<div class="flex justify-between items-center"><span>${c.username}</span><span class="text-indigo-400 font-mono">${c.pr_count} PRs</span></div>` | ||
| ).join('') || '<div class="text-slate-500">No data yet</div>'; |
There was a problem hiding this comment.
These UI updates build HTML via innerHTML using API-provided values (e.g., c.username). If any upstream data contains HTML, this becomes an XSS vector in the dashboard. Prefer creating DOM nodes and assigning textContent (or escape/sanitize the values before interpolation).
| const surveys = await fetch(API+'/surveys/active').then(r=>r.json()).catch(()=>[]); | ||
| document.getElementById('surveys-list').innerHTML = surveys.map(s => | ||
| `<div class="card p-4 flex justify-between items-center"> | ||
| <div><span class="font-medium">${s.title}</span><span class="text-slate-400 ml-2 text-sm">${s.total_questions} questions</span></div> | ||
| <span class="px-2 py-0.5 rounded text-xs bg-green-900 text-green-300">${s.status}</span> |
There was a problem hiding this comment.
surveys-list is populated via innerHTML with API-provided s.title/s.status, which can allow XSS if those fields contain markup. Build the DOM with textContent (or sanitize/escape) rather than interpolating into HTML strings.
| for pr in repo.get_pulls(state="all", sort="updated", direction="desc"): | ||
| created = pr.created_at.replace(tzinfo=timezone.utc) if pr.created_at.tzinfo is None else pr.created_at | ||
| if created < since_utc: | ||
| break | ||
| if created > until_utc: |
There was a problem hiding this comment.
The PR list is fetched sorted by "updated" but the loop breaks based on PR.created_at falling before since_utc. Because created_at is not monotonic when sorting by updated, this break can stop early and silently skip PRs that were created within the requested window but appear later in the updated-sorted pagination. Consider sorting by created (or using the Search API with a created: range) and only stopping pagination when the sort key indicates you’re past the window.
| async function runCollect() { | ||
| const btn = event.target; btn.textContent = 'Collecting...'; btn.disabled = true; | ||
| try { await fetch(API+'/metrics/snapshots'); } catch(e) {} | ||
| setTimeout(() => { btn.textContent = 'Collect'; btn.disabled = false; loadData(); }, 2000); | ||
| } |
There was a problem hiding this comment.
runCollect() relies on the implicit global event and performs a GET to /api/v1/metrics/snapshots, which only lists snapshots and does not trigger a collection run. Pass the event explicitly (or use this) and call a dedicated collection-triggering endpoint (e.g., POST /api/v1/metrics/collect / /snapshots/run) so the button actually starts collection.
|
|
||
| EXPOSE 8787 | ||
|
|
||
| CMD ["wellcode", "serve", "--host", "0.0.0.0", "--port", "8787"] |
There was a problem hiding this comment.
The container entrypoint starts the FastAPI server with wellcode serve --host 0.0.0.0 --port 8787, while the metrics and survey APIs are currently unauthenticated and served with very permissive CORS. When this image is run with a published port (e.g. via docker-compose), any host on the network can query detailed engineering metrics and survey data without authorization. Restrict the default bind address (or require an explicit host for non-local use) and/or add an authentication layer in the API so that exposing this container on 0.0.0.0 does not leak internal metrics by default.
| CMD ["wellcode", "serve", "--host", "0.0.0.0", "--port", "8787"] | |
| CMD ["sh", "-c", "wellcode serve --host ${WELLCODE_HOST:-127.0.0.1} --port ${WELLCODE_PORT:-8787}"] |
There was a problem hiding this comment.
Actionable comments posted: 12
Note
Due to the large number of review comments, Critical, Major severity comments were prioritized as inline comments.
🟡 Minor comments (11)
src/wellcode_cli/integrations/bitbucket/provider.py-112-114 (1)
112-114:⚠️ Potential issue | 🟡 MinorAuthor filtering uses
display_nameinstead ofusername, inconsistent with other providers.GitHub and GitLab providers filter by username/login, but this uses
display_name. If a caller passes a username (e.g., from a cross-provider query), no PRs will match. Usepr.get("author", {}).get("nickname", "")or"username"for consistency.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/wellcode_cli/integrations/bitbucket/provider.py` around lines 112 - 114, The Bitbucket PR author filter uses pr.get("author", {}).get("display_name", "") which mismatches other providers; update the check in provider.py (the pr_author variable and its comparison to author) to use a stable username field such as pr.get("author", {}).get("nickname", "") or pr.get("author", {}).get("username", "") so cross-provider username filtering works; ensure pr_author is derived from that field and the if author and pr_author != author: continue logic remains unchanged.src/wellcode_cli/api/routes/metrics.py-59-60 (1)
59-60:⚠️ Potential issue | 🟡 Minor
p.author.usernamethrowsAttributeErrorwhenauthor_idis NULL.If a
PullRequestMetrichas no linked author (author_id=None),p.authorisNone, and.usernamewill fail. Guard both the filter (line 60) and the contributor aggregation (line 73–74).Proposed fix
if author: - prs = [p for p in prs if p.author and p.author.username == author] + prs = [p for p in prs if p.author is not None and p.author.username == author] ... for p in prs: - if p.author: + if p.author is not None: contrib_counts[p.author.username] = contrib_counts.get(p.author.username, 0) + 1Note: The existing
if p.author:guard on line 73 works in practice since an ORM relationship returnsNone(falsy) for missing FKs, but being explicit aboutis not Noneis clearer for relationship objects.Also applies to: 72-74
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/wellcode_cli/api/routes/metrics.py` around lines 59 - 60, The filter and contributor-aggregation access nullable PullRequestMetric.author without guarding for None; update the list comprehension that filters by author to check "p.author is not None and p.author.username == author" (the comprehension that assigns prs) and change the contributor aggregation guard (the block that checks p.author before using p.author.username / adding to contributors) to use "p.author is not None" instead of a truthy check; this prevents AttributeError when author_id is NULL while preserving existing behavior.src/wellcode_cli/services/collector.py-145-147 (1)
145-147:⚠️ Potential issue | 🟡 Minor
cycle_time_hoursis assigned the same value aslead_time_hours.Line 147 sets
cycle_time_hours=lead_time(first_commit → merge), which is identical tolead_time_hourson line 146. If these are intended to be the same metric, remove one to avoid confusion. If cycle time should differ (e.g., include review time or PR-open → merge), compute it separately.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/wellcode_cli/services/collector.py` around lines 145 - 147, The code sets cycle_time_hours equal to lead_time_hours (cycle_time_hours=lead_time), which is either a redundant duplicate or the wrong metric; in the function/build of the metrics payload where coding_time_hours, lead_time_hours and cycle_time_hours are assigned, either remove cycle_time_hours if you intend only one metric, or compute cycle_time_hours separately (e.g., PR-open→merge or review_time = merge_time - pr_open_time, convert to hours) and pass that value instead; ensure the computed symbol (cycle_time_hours) uses the same units as coding_time_hours/lead_time_hours and update any callers/validators that consume these kwargs to expect the corrected metric name.src/wellcode_cli/services/ai_metrics.py-140-143 (1)
140-143:⚠️ Potential issue | 🟡 Minor"cursor" keyword matching may produce false positives.
"cursor" in text(line 142) will match any PR whose title or body contains the word "cursor" in any context (e.g., "fix cursor position", "database cursor"). Consider matching more specific patterns like "cursor ai" or "cursor-ai" or requiring the label match only.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/wellcode_cli/services/ai_metrics.py` around lines 140 - 143, The current heuristic line using if "cursor" in text or "cursor" in label_text (variables text and label_text) can produce false positives; change the logic that returns "cursor" so it matches stricter patterns (e.g., treat label_text equality/contains only, or use a regex to match whole words or specific tokens like "cursor ai" or "cursor-ai" or r'\bcursor(-ai| ai)?\b') and avoid broad substring checks in text; update the branch that returns "cursor" to validate with that regex or require label_text to match rather than any substring in text.src/wellcode_cli/db/models.py-108-118 (1)
108-118:⚠️ Potential issue | 🟡 MinorMissing unique constraint on
(team_id, developer_id)allows duplicate memberships.Without a constraint, calling
get_or_createon the team and then separately adding members could insert the same developer into the same team multiple times.Proposed fix
team = relationship("Team", back_populates="members") developer = relationship("Developer", back_populates="team_memberships") + + __table_args__ = ( + UniqueConstraint("team_id", "developer_id", name="uq_team_developer"), + )🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/wellcode_cli/db/models.py` around lines 108 - 118, Add a unique constraint to prevent duplicate memberships by updating the TeamMember model: in the TeamMember class (tablename "team_members") add a class-level __table_args__ that defines a UniqueConstraint on ("team_id", "developer_id") so the combination of team_id and developer_id is enforced as unique; ensure you import and use sqlalchemy.schema.UniqueConstraint and run/update the corresponding DB migration so the constraint is applied at the database level.src/wellcode_cli/integrations/gitlab/provider.py-137-186 (1)
137-186:⚠️ Potential issue | 🟡 MinorMissing
duration_secondscomputation for deployments.
SCMDeployment.duration_secondsis left at its default (None) even when bothdeployed_atandcompleted_atare available. The GitHub provider computes this. Add the calculation for consistency.Proposed fix
+ duration = None + if completed_at and deployed_at: + duration = (completed_at - deployed_at).total_seconds() + results.append(SCMDeployment( provider="gitlab", external_id=str(d.id), environment=d.environment, ref=d.ref, sha=d.sha, status=d.status, deployed_at=deployed_at, completed_at=completed_at, + duration_seconds=duration, is_rollback=False,🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/wellcode_cli/integrations/gitlab/provider.py` around lines 137 - 186, get_deployments builds SCMDeployment objects but never sets duration_seconds; when both deployed_at and completed_at are present compute duration_seconds = (completed_at - deployed_at).total_seconds() (or cast to int if other providers use ints) and pass it into the SCMDeployment constructor so duration_seconds is populated consistently; update the creation of SCMDeployment in get_deployments to calculate and include this field using the local deployed_at and completed_at variables.src/wellcode_cli/api/routes/metrics.py-68-68 (1)
68-68:⚠️ Potential issue | 🟡 MinorPotential
TypeErrorifadditionsordeletionsisNone.
p.additions + p.deletionswill throw if either column isNone. Use(p.additions or 0) + (p.deletions or 0)or filter outNoneexplicitly.Proposed fix
- sizes = [p.additions + p.deletions for p in prs if p.additions or p.deletions] + sizes = [(p.additions or 0) + (p.deletions or 0) for p in prs if p.additions or p.deletions]🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/wellcode_cli/api/routes/metrics.py` at line 68, The list comprehension computing sizes can raise TypeError when p.additions or p.deletions is None; update the expression that builds sizes in the route (variable sizes from prs) to use safe defaults e.g. treat None as 0 (replace p.additions + p.deletions with (p.additions or 0) + (p.deletions or 0)) or explicitly filter out PRs with None before summing so the sizes list is always numeric.src/wellcode_cli/integrations/bitbucket/provider.py-163-166 (1)
163-166:⚠️ Potential issue | 🟡 Minor
/1.0/groups/{workspace}endpoint is deprecated and unreliable.Bitbucket Cloud retired the 1.0 API in April 2019. As of February 2026, this GET-only endpoint is disabled on workspaces using Atlassian Cloud Admin and may return 404 or errors. Bitbucket's 2.0 API does not provide a direct replacement for groups management; consider migrating to the Atlassian Cloud Admin API for group operations, or use
/2.0/workspaces/{workspace}/membersand/2.0/workspaces/{workspace}/permissionsfor member/permission visibility.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/wellcode_cli/integrations/bitbucket/provider.py` around lines 163 - 166, The code calls the deprecated Bitbucket 1.0 groups endpoint using self.client.get(f"/1.0/groups/{self._workspace}"), which can return 404 or be disabled; replace that call with the 2.0 alternatives or the Atlassian Cloud Admin API: use self.client.get(f"/2.0/workspaces/{self._workspace}/members") and/or self.client.get(f"/2.0/workspaces/{self._workspace}/permissions") to retrieve membership/permission info, or integrate the Atlassian Cloud Admin API for group management, and add robust error handling around the new calls (handle 404/403 and fall back gracefully) where the original call is made.src/wellcode_cli/integrations/github/provider.py-180-183 (1)
180-183:⚠️ Potential issue | 🟡 MinorUse
continueinstead ofbreakto avoid skipping deployments.The GitHub REST API does not document or guarantee deployment ordering—the
GET /repos/{owner}/{repo}/deploymentsendpoint has nosortparameter. The current code assumes deployments are sorted newest-first; if the API returns them in a different order, valid deployments within the time range will be skipped. Replace thebreakstatement withcontinueto evaluate all deployments against thesince_utcanduntil_utcfilters.Proposed fix
if created < since_utc: - break + continue🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/wellcode_cli/integrations/github/provider.py` around lines 180 - 183, The loop over repo.get_deployments() wrongly uses break when a deployment's created_at is before since_utc, which can skip valid later deployments if the API order isn't newest-first; in the loop that checks deploy.created_at against since_utc and until_utc (refs: repo.get_deployments(), deploy.created_at, since_utc, until_utc), replace the break with continue so the loop evaluates all deployments and only skips the current out-of-range item, ensuring all candidates are filtered rather than prematurely terminating the iteration.src/wellcode_cli/services/dora.py-162-165 (1)
162-165:⚠️ Potential issue | 🟡 Minor
hotfixesis computed but excluded fromfailure_count— clarify intent.
revertsare counted as failures (line 164) buthotfixesare not, yet both indicate a change introduced a defect. If the exclusion is intentional, add an inline comment explaining the rationale; otherwise, consider adding+= hotfixesalongside+= reverts.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/wellcode_cli/services/dora.py` around lines 162 - 165, The code computes reverts and hotfixes from merged_prs but only adds reverts to failure_count; decide intent and fix accordingly: if hotfixes should count as failures, update the aggregation to also add hotfixes to failure_count (i.e., include hotfixes alongside reverts when updating failure_count), otherwise add an inline comment next to the hotfixes/reverts calculation explaining why hotfixes are intentionally excluded; reference the variables reverts, hotfixes, failure_count and the merged_prs comprehension in services/dora.py to locate the change.src/wellcode_cli/services/dora.py-153-153 (1)
153-153:⚠️ Potential issue | 🟡 MinorZero-fallback for missing lead-time/MTTR data produces inflated elite scores.
When no lead time data or no incidents exist, both
median_lead_timeandmttrdefault to0, which satisfies the<= 1helite threshold inclassify_dora_level. A period with no timing data should not score higher than a period with measured data.Consider using
Noneas the no-data sentinel and excluding metrics with no data from theclassify_dora_levelaverage (treating them as neutral, e.g., score 2.5) rather than coercing them to 0.Also applies to: 177-177
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/wellcode_cli/services/dora.py` at line 153, The current code coerces missing lead-time/MTTR values to 0 (e.g., median_lead_time = statistics.median(lead_times) if lead_times else 0), which incorrectly satisfies the elite <=1h threshold; change the sentinel for no-data to None (e.g., set median_lead_time and mttr to None when lists are empty) and update classify_dora_level to ignore None-valued metrics when computing the average score: treat absent metrics as neutral by substituting a midpoint score (2.5) only for the final average (or simply compute the mean over present metric scores), and ensure classify_dora_level handles None inputs for the lead_time, mttr, change_failure_rate, and deployment_frequency parameters without treating None as zero.
🧹 Nitpick comments (5)
src/wellcode_cli/services/collector.py (2)
196-201: Accessing privateprovider._orgcouples the collector toGitHubProviderinternals.
hasattr(provider, "_org")breaks the provider protocol abstraction. Consider adding anorganization_nameproperty to theSCMProviderprotocol so all providers can optionally surface this.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/wellcode_cli/services/collector.py` around lines 196 - 201, The code accesses the private attribute provider._org (in the loop over teams) which couples the collector to GitHubProvider internals; instead extend the SCMProvider interface to expose an optional organization_name property and update the collector to call provider.organization_name (falling back to None) before calling store.organizations.get_or_create with provider.provider_name; modify the concrete GitHubProvider to implement organization_name (returning its _org) so callers like the loop that invokes store.organizations.get_or_create no longer reference private attributes.
117-120: Move import out of the per-PR loop.
from .ai_metrics import detect_ai_tool_from_pris inside the loop body (line 117). While Python caches module imports, placing it at the top of the function (or file) is cleaner and makes dependencies explicit.Proposed fix
+from .ai_metrics import detect_ai_tool_from_pr + def collect_all( period_start: datetime, ...🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/wellcode_cli/services/collector.py` around lines 117 - 120, The import for detect_ai_tool_from_pr is inside the per-PR loop; move "from .ai_metrics import detect_ai_tool_from_pr" out of the loop to make the dependency explicit — place it at the top of the module or at the top of the enclosing function that iterates over SCM PRs (the loop that references scm_pr, ai_tool), and then call detect_ai_tool_from_pr(scm_pr.title, scm_pr.labels) inside the loop; if a circular import prevents moving it to module scope, move it to the function scope once (before the loop) instead of inside each iteration.src/wellcode_cli/integrations/github/provider.py (1)
40-72:get_repositories,get_pull_requests, andget_deploymentsall repeat the same repo-list resolution logic.Lines 42–46, 67–72, and 168–173 duplicate the same "resolve repo_list from org / user / specific repo" pattern. Extract a helper to reduce repetition.
Proposed refactor
+ def _resolve_repos(self, repo_full_name: Optional[str] = None): + if repo_full_name: + return [self.client.get_repo(repo_full_name)] + elif self._org: + return list(self.client.get_organization(self._org).get_repos()) + else: + return list(self.client.get_user().get_repos())🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/wellcode_cli/integrations/github/provider.py` around lines 40 - 72, The repo-resolution logic is duplicated across get_repositories, get_pull_requests, and get_deployments; extract a private helper (e.g., _resolve_repo_list or _get_repo_list) that accepts an optional repo_full_name and returns a list/iterable of GitHub repo objects by calling self.client.get_repo(repo_full_name) when provided, or self.client.get_organization(self._org).get_repos() when self._org is set, or self.client.get_user().get_repos() otherwise; add appropriate return type hints (list or Iterable) and update get_repositories, get_pull_requests, and get_deployments to call this helper instead of repeating the conditional logic.src/wellcode_cli/db/repository.py (1)
134-152: Upsert overwrites existing values with column defaults (0, False) when they appear on the incoming object.The
val is not Noneguard (line 146) cannot distinguish between "field was explicitly set to 0/False" and "field has a default value of 0/False because it wasn't populated." If a provider supplies fewer fields (e.g., Bitbucket doesn't provideadditions), the incomingPullRequestMetricwithadditions=0will overwrite a previously stored non-zero value.Consider either accepting the current behavior as a known trade-off, or switching to a more explicit approach (e.g., tracking which fields were explicitly set).
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/wellcode_cli/db/repository.py` around lines 134 - 152, The upsert in PullRequestRepo.upsert is overwriting stored values when incoming PullRequestMetric fields hold default falsy values (0/False) because it only checks val is not None; change the update guard to detect fields explicitly set on the incoming object (e.g., check if col.name is present in pr.__dict__) and only assign when the attribute was provided by the caller (use pr.__dict__.get(col.name) existence check rather than val is not None), then set existing.<col.name> = getattr(pr, col.name) for those explicitly provided fields.src/wellcode_cli/api/routes/surveys.py (1)
90-98: Potential N+1 query:s.questionsaccessed per-survey in a loop.If the
Survey.questionsrelationship is lazy-loaded (the default for SQLAlchemy), each iteration emits a separateSELECTagainstsurvey_questions. With many active surveys this becomes a hot-path issue.Consider eager-loading the relationship in
get_active_surveys(), e.g.:# In SurveyRepo.get_active_surveys(): from sqlalchemy.orm import joinedload q = select(Survey).where(Survey.status == "active").options(joinedload(Survey.questions))🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/wellcode_cli/api/routes/surveys.py` around lines 90 - 98, The loop in surveys.py accesses s.questions per-survey causing an N+1 query; update the data fetch to eager-load the relationship in the repository (e.g., SurveyRepo.get_active_surveys) so questions are loaded with the surveys (use sqlalchemy.orm.joinedload or selectinload on Survey.questions in the select/query that returns surveys) and keep the loop in surveys.py as-is to use the already-populated s.questions when building SurveyResponseModel.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/wellcode_cli/api/routes/metrics.py`:
- Around line 54-82: The session opened via get_session() in get_pr_metrics (and
similarly in list_snapshots) can leak if an exception occurs; refactor so the
MetricStore(session) usage is wrapped in a try/finally (or use a context
manager) and call session.close() in the finally block to guarantee cleanup;
locate get_pr_metrics and list_snapshots, ensure all early returns move any
computed values to local variables and perform session.close() in finally (or
use "with get_session() as session" if supported) and keep references to
MetricStore, prs, merged, open_prs, and contrib_counts intact.
In `@src/wellcode_cli/api/routes/surveys.py`:
- Around line 85-100: The session opened by get_session() in list_active_surveys
can leak if an exception occurs when calling
MetricStore(...).surveys.get_active_surveys() or while constructing
SurveyResponseModel instances; wrap the session usage in a try/finally (or use a
context manager if get_session returns one) so session.close() is always
executed, e.g., acquire session, create MetricStore and call
store.surveys.get_active_surveys() inside try, build the result list, and call
session.close() in finally; ensure references to list_active_surveys,
get_session, MetricStore, store.surveys.get_active_surveys, and
SurveyResponseModel are preserved when moving code into the guarded block.
In `@src/wellcode_cli/db/models.py`:
- Around line 85-105: The Developer.email Column currently stores PII but is
unused in this PR; either remove it from the Developer model (delete the email =
Column(...) line in class Developer and add a DB migration to drop the column
and update any code referencing Developer.email), or if the field must be
retained, replace it with an encrypted column (e.g., use
sqlalchemy_utils.EncryptedType or your project's secrets/encryption helper) and
mark it clearly as sensitive (nullable if not always populated), add/adjust
migration, and ensure access controls/audit/retention policy are implemented for
reads/writes to Developer.email.
In `@src/wellcode_cli/integrations/bitbucket/provider.py`:
- Around line 60-64: get_repositories and get_pull_requests currently only fetch
a single page (using pagelen) and drop additional pages; change both to loop
following Bitbucket's pagination by collecting results from the initial response
and then while data.get("next") is truthy call self.client.get(next_url)
(preserving any needed params for the first call), extend/append the returned
"values" into an accumulator list, and return the full aggregated list; ensure
you reference self.client.get, data.get("next") and the "values" field when
implementing the loop so all pages are fetched and merged.
- Around line 107-110: The comparisons against created_at currently use
since.replace(tzinfo=timezone.utc) and until.replace(tzinfo=timezone.utc) which
can silently change timezone-aware datetimes; instead compute normalized
since_utc and until_utc once before the loop: for each of since and until, if
.tzinfo is None use .replace(tzinfo=timezone.utc) else use
.astimezone(timezone.utc), then use those since_utc/until_utc variables in the
created_at checks (the code around the created_at comparisons) to avoid
per-iteration recomputation and preserve correct timezone conversion.
In `@src/wellcode_cli/integrations/github/provider.py`:
- Around line 110-113: first_review_at calculation can raise ValueError when all
reviews have submitted_at==None because min() receives an empty generator;
change the logic in the block that computes first_review_at (the reviews ->
first_review_at code) to first filter or construct a list of valid datetimes
(e.g. dates = [r.submitted_at for r in reviews if r.submitted_at]) and only call
min(dates) if dates is non-empty (or use min(dates, default=None)); then apply
the existing tzinfo normalization (replace(tzinfo=timezone.utc) if fr.tzinfo is
None else fr) to the min result and assign to first_review_at, ensuring
first_review_at stays None when no valid submitted_at exists.
In `@src/wellcode_cli/main.py`:
- Around line 150-153: The DB session created by get_session() is closed only on
the happy path, so if compute_dora or compute_ai_impact raises the connection is
leaked; wrap session usage in a try/finally (or use a contextmanager) so
session.close() always runs: e.g., create session = get_session(), instantiate
MetricStore(session) and call compute_dora(...) inside try, then in finally call
session.close(); apply the same pattern to the ai_metrics_cmd flow that calls
compute_ai_impact to ensure no leaked DB sessions.
- Around line 88-103: The CLI accepts tz-naive datetimes from click.DateTime
which causes mixed-aware/naive arithmetic in collect (function collect using
start_date/end_date); normalize any incoming start_date and end_date to UTC
immediately after parsing: if a date is tz-naive, attach timezone.utc (or
convert to UTC) so both start_date and end_date are timezone-aware before any
timedelta arithmetic; apply the same normalization pattern to the equivalent
date inputs in dora (where period_start/period_end are used) and ai_metrics_cmd
to prevent offset-naive vs offset-aware TypeError.
In `@src/wellcode_cli/services/ai_metrics.py`:
- Around line 75-104: The code currently calls the deprecated Copilot /usage
endpoint and expects flat total_* fields; update the request URL in the block
where url is built (used in the try block that creates AIUsageMetric and calls
store.ai_usage.add) to "https://api.github.com/orgs/{org}/copilot/metrics" and
change the parsing logic to traverse the new nested metric groups (e.g.,
"copilot_ide_code_completions", "copilot_ide_chat", "copilot_dotcom_chat",
"copilot_dotcom_pull_requests") for each day's entry: extract the appropriate
counters from each group's metrics (map completions/chat metrics to
suggestions_shown, suggestions_accepted, lines_suggested, lines_accepted,
active_users, chat_sessions as applicable), preserve date parsing
(datetime.fromisoformat(...).replace(tzinfo=timezone.utc)), construct
AIUsageMetric with the mapped values and keep calling
store.ai_usage.add(metric), metrics.append(metric) and store.commit() as before;
ensure missing groups/fields default to 0 and include the original breakdown (or
store the full group structure) under metadata_.
In `@src/wellcode_cli/services/collector.py`:
- Around line 186-191: The collector currently calls
store.deployments.add(deploy_metric) which always inserts and causes duplicate
DeploymentMetric rows on repeated runs; add an upsert for deployments similar to
PullRequestRepo.upsert that checks for an existing record by (provider,
external_id) and updates it instead of inserting, or alternatively add a unique
constraint on DeploymentMetric for (provider, external_id) and use
session.merge()/upsert logic in DeploymentRepo; update DeploymentRepo to expose
an upsert_deployment(deployment_metric) (or upsert) and replace
store.deployments.add(...) in the collector (the code creating deploy_metric and
calling store.deployments.add) with the new upsert call so re-runs do not create
duplicates.
In `@src/wellcode_cli/services/dora.py`:
- Around line 108-114: compute_dora accepts a team_id but never uses it when
querying data, so callers get org-wide metrics; update compute_dora to forward
team_id to all relevant repository queries (store.deployments.get_by_period,
store.pull_requests.get_merged_by_period, store.incidents.get_by_period) and any
other store.* calls used to build DORAMetrics, and also persist the same team_id
in the DORASnapshot creation path; ensure function signatures and calls pass
team_id (or None) consistently so team-scoped metrics are actually returned.
- Around line 182-213: The DORAMetrics uses a clamped change_failure_rate
(min(cfr, 1.0)) but DORASnapshot persists the raw cfr, causing divergence; fix
by persisting the same clamped value instead of raw cfr—use the clamped value
(e.g., metrics.change_failure_rate or a local clamped_cfr = min(cfr, 1.0)) when
constructing DORASnapshot so DORAMetrics and DORASnapshot remain consistent.
---
Duplicate comments:
In `@src/wellcode_cli/integrations/github/provider.py`:
- Around line 79-84: The loop over repo.get_pulls is breaking when created <
since_utc while the pulls are sorted by "updated", which can skip newer-created
PRs that were updated; fix by either changing the get_pulls call to
sort="created" (so a break is valid) or (safer) replace the break with continue
so you only skip that PR but keep iterating; adjust the call or the control flow
around repo.get_pulls, the created variable, since_utc and until_utc
accordingly.
---
Nitpick comments:
In `@src/wellcode_cli/api/routes/surveys.py`:
- Around line 90-98: The loop in surveys.py accesses s.questions per-survey
causing an N+1 query; update the data fetch to eager-load the relationship in
the repository (e.g., SurveyRepo.get_active_surveys) so questions are loaded
with the surveys (use sqlalchemy.orm.joinedload or selectinload on
Survey.questions in the select/query that returns surveys) and keep the loop in
surveys.py as-is to use the already-populated s.questions when building
SurveyResponseModel.
In `@src/wellcode_cli/db/repository.py`:
- Around line 134-152: The upsert in PullRequestRepo.upsert is overwriting
stored values when incoming PullRequestMetric fields hold default falsy values
(0/False) because it only checks val is not None; change the update guard to
detect fields explicitly set on the incoming object (e.g., check if col.name is
present in pr.__dict__) and only assign when the attribute was provided by the
caller (use pr.__dict__.get(col.name) existence check rather than val is not
None), then set existing.<col.name> = getattr(pr, col.name) for those explicitly
provided fields.
In `@src/wellcode_cli/integrations/github/provider.py`:
- Around line 40-72: The repo-resolution logic is duplicated across
get_repositories, get_pull_requests, and get_deployments; extract a private
helper (e.g., _resolve_repo_list or _get_repo_list) that accepts an optional
repo_full_name and returns a list/iterable of GitHub repo objects by calling
self.client.get_repo(repo_full_name) when provided, or
self.client.get_organization(self._org).get_repos() when self._org is set, or
self.client.get_user().get_repos() otherwise; add appropriate return type hints
(list or Iterable) and update get_repositories, get_pull_requests, and
get_deployments to call this helper instead of repeating the conditional logic.
In `@src/wellcode_cli/services/collector.py`:
- Around line 196-201: The code accesses the private attribute provider._org (in
the loop over teams) which couples the collector to GitHubProvider internals;
instead extend the SCMProvider interface to expose an optional organization_name
property and update the collector to call provider.organization_name (falling
back to None) before calling store.organizations.get_or_create with
provider.provider_name; modify the concrete GitHubProvider to implement
organization_name (returning its _org) so callers like the loop that invokes
store.organizations.get_or_create no longer reference private attributes.
- Around line 117-120: The import for detect_ai_tool_from_pr is inside the
per-PR loop; move "from .ai_metrics import detect_ai_tool_from_pr" out of the
loop to make the dependency explicit — place it at the top of the module or at
the top of the enclosing function that iterates over SCM PRs (the loop that
references scm_pr, ai_tool), and then call detect_ai_tool_from_pr(scm_pr.title,
scm_pr.labels) inside the loop; if a circular import prevents moving it to
module scope, move it to the function scope once (before the loop) instead of
inside each iteration.
| session = get_session() | ||
| store = MetricStore(session) | ||
|
|
||
| prs = store.pull_requests.get_by_period(start_dt, end_dt, repo_id=repo_id) | ||
|
|
||
| if author: | ||
| prs = [p for p in prs if p.author and p.author.username == author] | ||
|
|
||
| merged = [p for p in prs if p.state == "merged"] | ||
| open_prs = [p for p in prs if p.state == "open"] | ||
|
|
||
| cycle_times = [p.cycle_time_hours for p in merged if p.cycle_time_hours] | ||
| merge_times = [p.time_to_merge_hours for p in merged if p.time_to_merge_hours] | ||
| review_times = [p.time_to_first_review_hours for p in prs if p.time_to_first_review_hours] | ||
| sizes = [p.additions + p.deletions for p in prs if p.additions or p.deletions] | ||
|
|
||
| # Top contributors | ||
| contrib_counts = {} | ||
| for p in prs: | ||
| if p.author: | ||
| contrib_counts[p.author.username] = contrib_counts.get(p.author.username, 0) + 1 | ||
| top = sorted(contrib_counts.items(), key=lambda x: x[1], reverse=True)[:10] | ||
|
|
||
| # Size distribution | ||
| small = sum(1 for s in sizes if s < 100) | ||
| medium = sum(1 for s in sizes if 100 <= s < 500) | ||
| large = sum(1 for s in sizes if s >= 500) | ||
|
|
||
| session.close() |
There was a problem hiding this comment.
Session is not closed on exception — resource leak.
Both get_pr_metrics and list_snapshots open a session but only close it in the happy path. Any exception between get_session() and session.close() leaks the connection. Use try/finally or a context manager.
Proposed fix for get_pr_metrics
session = get_session()
- store = MetricStore(session)
-
- prs = store.pull_requests.get_by_period(start_dt, end_dt, repo_id=repo_id)
- ...
- session.close()
-
- return PRMetricsResponse(...)
+ try:
+ store = MetricStore(session)
+ prs = store.pull_requests.get_by_period(start_dt, end_dt, repo_id=repo_id)
+ ...
+ return PRMetricsResponse(...)
+ finally:
+ session.close()Also applies to: 103-119
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/wellcode_cli/api/routes/metrics.py` around lines 54 - 82, The session
opened via get_session() in get_pr_metrics (and similarly in list_snapshots) can
leak if an exception occurs; refactor so the MetricStore(session) usage is
wrapped in a try/finally (or use a context manager) and call session.close() in
the finally block to guarantee cleanup; locate get_pr_metrics and
list_snapshots, ensure all early returns move any computed values to local
variables and perform session.close() in finally (or use "with get_session() as
session" if supported) and keep references to MetricStore, prs, merged,
open_prs, and contrib_counts intact.
| def list_active_surveys(): | ||
| session = get_session() | ||
| store = MetricStore(session) | ||
| surveys = store.surveys.get_active_surveys() | ||
| result = [] | ||
| for s in surveys: | ||
| result.append(SurveyResponseModel( | ||
| id=s.id, | ||
| title=s.title, | ||
| survey_type=s.survey_type, | ||
| status=s.status, | ||
| created_at=s.created_at.isoformat() if s.created_at else "", | ||
| total_questions=len(s.questions) if s.questions else 0, | ||
| )) | ||
| session.close() | ||
| return result |
There was a problem hiding this comment.
Session is not closed if an exception occurs inside list_active_surveys.
session.close() at line 99 is only reached on the happy path. An exception in get_active_surveys() or while building SurveyResponseModel objects leaks the connection.
🐛 Proposed fix
def list_active_surveys():
session = get_session()
- store = MetricStore(session)
- surveys = store.surveys.get_active_surveys()
- result = []
- for s in surveys:
- result.append(SurveyResponseModel(
- id=s.id,
- title=s.title,
- survey_type=s.survey_type,
- status=s.status,
- created_at=s.created_at.isoformat() if s.created_at else "",
- total_questions=len(s.questions) if s.questions else 0,
- ))
- session.close()
- return result
+ try:
+ store = MetricStore(session)
+ surveys = store.surveys.get_active_surveys()
+ result = [
+ SurveyResponseModel(
+ id=s.id,
+ title=s.title,
+ survey_type=s.survey_type,
+ status=s.status,
+ created_at=s.created_at.isoformat() if s.created_at else "",
+ total_questions=len(s.questions) if s.questions else 0,
+ )
+ for s in surveys
+ ]
+ finally:
+ session.close()
+ return result🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/wellcode_cli/api/routes/surveys.py` around lines 85 - 100, The session
opened by get_session() in list_active_surveys can leak if an exception occurs
when calling MetricStore(...).surveys.get_active_surveys() or while constructing
SurveyResponseModel instances; wrap the session usage in a try/finally (or use a
context manager if get_session returns one) so session.close() is always
executed, e.g., acquire session, create MetricStore and call
store.surveys.get_active_surveys() inside try, build the result list, and call
session.close() in finally; ensure references to list_active_surveys,
get_session, MetricStore, store.surveys.get_active_surveys, and
SurveyResponseModel are preserved when moving code into the guarded block.
| class Developer(Base): | ||
| __tablename__ = "developers" | ||
|
|
||
| id = Column(Integer, primary_key=True, autoincrement=True) | ||
| username = Column(String(255), nullable=False) | ||
| display_name = Column(String(255)) | ||
| email = Column(String(255)) | ||
| provider = Column(String(50), nullable=False) | ||
| external_id = Column(String(255)) | ||
| avatar_url = Column(Text) | ||
| created_at = Column(DateTime, default=lambda: datetime.now(timezone.utc)) | ||
| updated_at = Column(DateTime, default=lambda: datetime.now(timezone.utc), | ||
| onupdate=lambda: datetime.now(timezone.utc)) | ||
|
|
||
| team_memberships = relationship("TeamMember", back_populates="developer") | ||
| pull_requests = relationship("PullRequestMetric", back_populates="author", | ||
| foreign_keys="PullRequestMetric.author_id") | ||
|
|
||
| __table_args__ = ( | ||
| UniqueConstraint("provider", "username", name="uq_dev_provider_username"), | ||
| ) |
There was a problem hiding this comment.
Developer email column stores PII — ensure it's necessary and protected.
This PR is labeled security-sensitive. The email field (line 91) stores personally identifiable information. Consider whether this field is actually needed (it's never populated by any provider in this PR). If retained, ensure it's subject to data retention policies and access controls for GDPR/CCPA compliance.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/wellcode_cli/db/models.py` around lines 85 - 105, The Developer.email
Column currently stores PII but is unused in this PR; either remove it from the
Developer model (delete the email = Column(...) line in class Developer and add
a DB migration to drop the column and update any code referencing
Developer.email), or if the field must be retained, replace it with an encrypted
column (e.g., use sqlalchemy_utils.EncryptedType or your project's
secrets/encryption helper) and mark it clearly as sensitive (nullable if not
always populated), add/adjust migration, and ensure access
controls/audit/retention policy are implemented for reads/writes to
Developer.email.
| try: | ||
| data = self.client.get( | ||
| f"/2.0/repositories/{self._workspace}", | ||
| params={"pagelen": 100}, | ||
| ) |
There was a problem hiding this comment.
Only the first page of results is fetched — no pagination handling.
get_repositories (line 61–64) and get_pull_requests (line 96–99) each fetch a single page via pagelen. Workspaces/repos with more items than the page size will silently lose data. Iterate through data.get("next") links to collect all pages.
Also applies to: 96-99
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/wellcode_cli/integrations/bitbucket/provider.py` around lines 60 - 64,
get_repositories and get_pull_requests currently only fetch a single page (using
pagelen) and drop additional pages; change both to loop following Bitbucket's
pagination by collecting results from the initial response and then while
data.get("next") is truthy call self.client.get(next_url) (preserving any needed
params for the first call), extend/append the returned "values" into an
accumulator list, and return the full aggregated list; ensure you reference
self.client.get, data.get("next") and the "values" field when implementing the
loop so all pages are fetched and merged.
| if created_at < since.replace(tzinfo=timezone.utc): | ||
| break | ||
| if created_at > until.replace(tzinfo=timezone.utc): | ||
| continue |
There was a problem hiding this comment.
since.replace(tzinfo=timezone.utc) unconditionally overwrites any existing timezone.
If since is already timezone-aware (e.g., non-UTC), .replace(tzinfo=timezone.utc) silently changes its meaning without converting. The GitHub provider handles this correctly with a conditional check. Apply the same pattern here.
Proposed fix
- if created_at < since.replace(tzinfo=timezone.utc):
+ since_utc = since.replace(tzinfo=timezone.utc) if since.tzinfo is None else since
+ until_utc = until.replace(tzinfo=timezone.utc) if until.tzinfo is None else until
+ if created_at < since_utc:
break
- if created_at > until.replace(tzinfo=timezone.utc):
+ if created_at > until_utc:
continueMove the since_utc/until_utc computation outside the inner loop (before line 94) to avoid recomputing every iteration.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/wellcode_cli/integrations/bitbucket/provider.py` around lines 107 - 110,
The comparisons against created_at currently use
since.replace(tzinfo=timezone.utc) and until.replace(tzinfo=timezone.utc) which
can silently change timezone-aware datetimes; instead compute normalized
since_utc and until_utc once before the loop: for each of since and until, if
.tzinfo is None use .replace(tzinfo=timezone.utc) else use
.astimezone(timezone.utc), then use those since_utc/until_utc variables in the
created_at checks (the code around the created_at comparisons) to avoid
per-iteration recomputation and preserve correct timezone conversion.
| session = get_session() | ||
| store = MetricStore(session) | ||
| metrics = compute_dora(store, start_date, end_date, repo_id=repo_id, team_id=team_id) | ||
| session.close() |
There was a problem hiding this comment.
DB session is never closed if compute_dora / compute_ai_impact raises an exception.
Both dora and ai_metrics_cmd call session.close() only on the happy path. Any DB or service error leaks the connection back to the pool.
🐛 Proposed fix (shown for `dora`; apply the same pattern to `ai_metrics_cmd`)
session = get_session()
store = MetricStore(session)
- metrics = compute_dora(store, start_date, end_date, repo_id=repo_id, team_id=team_id)
- session.close()
+ try:
+ metrics = compute_dora(store, start_date, end_date, repo_id=repo_id, team_id=team_id)
+ finally:
+ session.close()Also applies to: 217-220
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/wellcode_cli/main.py` around lines 150 - 153, The DB session created by
get_session() is closed only on the happy path, so if compute_dora or
compute_ai_impact raises the connection is leaked; wrap session usage in a
try/finally (or use a contextmanager) so session.close() always runs: e.g.,
create session = get_session(), instantiate MetricStore(session) and call
compute_dora(...) inside try, then in finally call session.close(); apply the
same pattern to the ai_metrics_cmd flow that calls compute_ai_impact to ensure
no leaked DB sessions.
| try: | ||
| url = f"https://api.github.com/orgs/{org}/copilot/usage" | ||
| params = {"since": since_str, "until": until_str} | ||
| resp = requests.get(url, headers=headers, params=params, timeout=30) | ||
|
|
||
| if resp.status_code == 200: | ||
| data = resp.json() | ||
| for day in data: | ||
| date_str = day.get("day", "") | ||
| if not date_str: | ||
| continue | ||
|
|
||
| metric = AIUsageMetric( | ||
| snapshot_id=snapshot_id, | ||
| tool="copilot", | ||
| date=datetime.fromisoformat(date_str).replace(tzinfo=timezone.utc), | ||
| suggestions_shown=day.get("total_suggestions_count", 0), | ||
| suggestions_accepted=day.get("total_acceptances_count", 0), | ||
| lines_suggested=day.get("total_lines_suggested", 0), | ||
| lines_accepted=day.get("total_lines_accepted", 0), | ||
| active_users=day.get("total_active_users", 0), | ||
| chat_sessions=day.get("total_chat_turns", 0), | ||
| metadata_={ | ||
| "breakdown": day.get("breakdown", []), | ||
| }, | ||
| ) | ||
| store.ai_usage.add(metric) | ||
| metrics.append(metric) | ||
|
|
||
| store.commit() |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
GitHub Copilot organization usage API response format documentation
💡 Result:
GitHub’s documentation for the organization-level Copilot “usage” data is now under the Copilot Metrics API (the older beta /usage endpoints were deprecated starting February 2025). [1], [2]
Official response format (organization scope)
Endpoint: GET /orgs/{org}/copilot/metrics [1]
Response: a JSON array of daily metric objects (up to ~100 days), e.g.:
-
Top-level fields per day:
date(YYYY-MM-DD)total_active_userstotal_engaged_users
-
Nested metric groups (each with its own
total_engaged_usersand breakdowns), including:copilot_ide_code_completions(bylanguages,editors→models→languageswith suggestion/acceptance/line counts)copilot_ide_chat(byeditors→modelswith chat/insertion/copy counts)copilot_dotcom_chat(bymodelswith chat counts)copilot_dotcom_pull_requests(byrepositories→modelswith PR summary counts)
The GitHub Docs page includes both an Example response and a Response schema tab with the detailed structure/definitions. [1]
Note on “/usage” endpoints
GitHub announced the beta Copilot /usage endpoints deprecation in February 2025 and recommends using the Copilot /metrics endpoints instead. [2]
Replace the deprecated Copilot /usage endpoint with the official /metrics endpoint.
The Copilot organization usage /usage endpoint was deprecated in February 2025 and may no longer be available. Line 76 must migrate to https://api.github.com/orgs/{org}/copilot/metrics, which has a significantly different response structure with nested metric groups (copilot_ide_code_completions, copilot_ide_chat, copilot_dotcom_chat, copilot_dotcom_pull_requests) instead of the flat total_* fields currently used in lines 83–91. The response is still a top-level JSON array, but the field names and nesting have changed.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/wellcode_cli/services/ai_metrics.py` around lines 75 - 104, The code
currently calls the deprecated Copilot /usage endpoint and expects flat total_*
fields; update the request URL in the block where url is built (used in the try
block that creates AIUsageMetric and calls store.ai_usage.add) to
"https://api.github.com/orgs/{org}/copilot/metrics" and change the parsing logic
to traverse the new nested metric groups (e.g., "copilot_ide_code_completions",
"copilot_ide_chat", "copilot_dotcom_chat", "copilot_dotcom_pull_requests") for
each day's entry: extract the appropriate counters from each group's metrics
(map completions/chat metrics to suggestions_shown, suggestions_accepted,
lines_suggested, lines_accepted, active_users, chat_sessions as applicable),
preserve date parsing
(datetime.fromisoformat(...).replace(tzinfo=timezone.utc)), construct
AIUsageMetric with the mapped values and keep calling
store.ai_usage.add(metric), metrics.append(metric) and store.commit() as before;
ensure missing groups/fields default to 0 and include the original breakdown (or
store the full group structure) under metadata_.
| pr_number=scm_deploy.pr_number, | ||
| ) | ||
| store.deployments.add(deploy_metric) | ||
|
|
||
| prov_summary["deployments"] = len(deploys) | ||
| store.commit() |
There was a problem hiding this comment.
Deployments are added without dedup — re-running collection duplicates rows.
store.deployments.add() always inserts. Unlike store.pull_requests.upsert() which checks for existing records by (provider, external_id), deployments have no such guard. Running collect_all twice for the same period will create duplicate DeploymentMetric rows.
Add an upsert method to DeploymentRepo (matching the pattern in PullRequestRepo) or add a unique constraint on (provider, external_id) in the model and use merge/upsert logic.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/wellcode_cli/services/collector.py` around lines 186 - 191, The collector
currently calls store.deployments.add(deploy_metric) which always inserts and
causes duplicate DeploymentMetric rows on repeated runs; add an upsert for
deployments similar to PullRequestRepo.upsert that checks for an existing record
by (provider, external_id) and updates it instead of inserting, or alternatively
add a unique constraint on DeploymentMetric for (provider, external_id) and use
session.merge()/upsert logic in DeploymentRepo; update DeploymentRepo to expose
an upsert_deployment(deployment_metric) (or upsert) and replace
store.deployments.add(...) in the collector (the code creating deploy_metric and
calling store.deployments.add) with the new upsert call so re-runs do not create
duplicates.
| def compute_dora( | ||
| store: MetricStore, | ||
| period_start: datetime, | ||
| period_end: datetime, | ||
| repo_id: Optional[int] = None, | ||
| team_id: Optional[int] = None, | ||
| ) -> DORAMetrics: |
There was a problem hiding this comment.
team_id parameter is accepted but silently ignored in all data queries.
team_id is only stored in the persisted DORASnapshot (line 204) but is never passed to store.deployments.get_by_period(), store.pull_requests.get_merged_by_period(), or store.incidents.get_by_period(). Callers invoking compute_dora(..., team_id=X) will always receive org-wide metrics, not team-scoped ones, with no indication that filtering failed.
#!/bin/bash
# Confirm team_id is not supported in any repository query method used here
rg -n "team_id" src/wellcode_cli/db/repository.py🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/wellcode_cli/services/dora.py` around lines 108 - 114, compute_dora
accepts a team_id but never uses it when querying data, so callers get org-wide
metrics; update compute_dora to forward team_id to all relevant repository
queries (store.deployments.get_by_period,
store.pull_requests.get_merged_by_period, store.incidents.get_by_period) and any
other store.* calls used to build DORAMetrics, and also persist the same team_id
in the DORASnapshot creation path; ensure function signatures and calls pass
team_id (or None) consistently so team-scoped metrics are actually returned.
| change_failure_rate=min(cfr, 1.0), | ||
| mttr_hours=mttr, | ||
| level="", | ||
| details={ | ||
| "total_deployments": len(deployments), | ||
| "successful_deployments": len(successful_deploys), | ||
| "failed_deployments": len(failed_deploys), | ||
| "total_merged_prs": len(merged_prs), | ||
| "reverts": reverts, | ||
| "hotfixes": hotfixes, | ||
| "incidents": len(incidents), | ||
| "change_incidents": len(change_incidents), | ||
| "lead_times_count": len(lead_times), | ||
| "recovery_times_count": len(recovery_times), | ||
| "period_days": days, | ||
| }, | ||
| ) | ||
| metrics.level = classify_dora_level(metrics) | ||
|
|
||
| # Persist the snapshot | ||
| dora_snap = DORASnapshot( | ||
| repository_id=repo_id, | ||
| team_id=team_id, | ||
| period_start=period_start, | ||
| period_end=period_end, | ||
| deployment_frequency=deploy_freq, | ||
| lead_time_hours=median_lead_time, | ||
| change_failure_rate=cfr, | ||
| mttr_hours=mttr, | ||
| level=metrics.level, | ||
| details=metrics.details, | ||
| ) |
There was a problem hiding this comment.
change_failure_rate is capped in DORAMetrics but stored raw in DORASnapshot — values diverge.
Line 182 clamps CFR to 1.0 (min(cfr, 1.0)), but line 209 writes the unclamped cfr to the database. If failure_count > total_deploys (e.g., many reverts in a low-deployment period), the persisted snapshot will hold a value > 1.0 while the in-memory object and all callers see ≤ 1.0.
🐛 Proposed fix
- change_failure_rate=cfr,
+ change_failure_rate=min(cfr, 1.0),🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/wellcode_cli/services/dora.py` around lines 182 - 213, The DORAMetrics
uses a clamped change_failure_rate (min(cfr, 1.0)) but DORASnapshot persists the
raw cfr, causing divergence; fix by persisting the same clamped value instead of
raw cfr—use the clamped value (e.g., metrics.change_failure_rate or a local
clamped_cfr = min(cfr, 1.0)) when constructing DORASnapshot so DORAMetrics and
DORASnapshot remain consistent.
2 participants
This pull request introduces a major update to the Wellcode CLI, adding a new FastAPI-based web dashboard and API for developer productivity metrics, along with Docker and deployment support. The changes include new API endpoints for DORA, PR metrics, AI coding tool metrics, and developer experience surveys, as well as updates to dependencies and configuration for modern Python and web frameworks.
Deployment & Infrastructure
Dockerfileanddocker-compose.ymlfor containerized deployment, including PostgreSQL integration and environment variable support for external services. [1] [2]Web API & Dashboard
src/wellcode_cli/api/app.py) with CORS middleware, static dashboard serving, and API routers for health, metrics, DORA, AI metrics, and surveys.API Endpoints
src/wellcode_cli/api/routes/dora.py)src/wellcode_cli/api/routes/metrics.py)src/wellcode_cli/api/routes/ai_metrics.py)src/wellcode_cli/api/routes/surveys.py)src/wellcode_cli/api/routes/health.py)Configuration & Dependency Updates
pyproject.tomlwith new dependencies (FastAPI, SQLAlchemy, Alembic, etc.), improved project metadata, and added test/dev tools. Also added Alembic configuration for migrations (alembic.ini). [1] [2] [3] [4] [5]Versioning
0.2.0and updated author information. [1] [2]These changes collectively enable Wellcode to be deployed as a modern web service, provide robust API endpoints for engineering metrics, and lay the foundation for future dashboard and analytics features.
Summary by CodeRabbit
New Features
Chores
Documentation