fix(vcs): use the mapped status when picking the comment image extension by Meldiron · Pull Request #13771 · appwrite/appwrite

Meldiron · 2026-09-19T09:49:05Z

What does this PR do?

Fixes the broken status image in Git PR comments for Sites while a deployment is processing:

In Appwrite\Vcs\Comment::generateComment(), the Sites branch maps processing onto the building image, but derived the file extension from the raw status:

$imageStatus = in_array($site['status'], ['processing', 'building']) ? 'building' : $site['status'];
$extension = $site['status'] === 'building' ? 'gif' : 'png';   // 'png' when processing

So a processing deployment requested /images/vcs/status-building-light.png, which we don't ship — only the .gif variants exist for building. Result: a broken image in every PR comment during the processing phase.

$ curl -o /dev/null -w '%{http_code}\n' https://cloud.appwrite.io/images/vcs/status-building-light.png
404
$ curl -o /dev/null -w '%{http_code}\n' https://cloud.appwrite.io/images/vcs/status-building-light.gif
200

The Functions branch a few lines below already keys off $imageStatus and is unaffected. This PR makes Sites match, and adds a unit test asserting every site status renders a path that actually exists in public/images/vcs/.

Introduced in 0ac6dec (shipped in 1.8.0), so it affects 1.8.x, 1.9.x, 2.0.x and main.

Test Plan

vendor/bin/phpunit tests/unit/Vcs/CommentTest.php — 5 tests, 30 assertions, passing.
The new testSiteStatusImagesMatchExistingFiles fails on main (asserts status-building-light.gif, gets .png) and passes with the fix.
composer lint clean on both touched files.

Related PRs and Issues

Regression from 0ac6dec

Checklist

Have you read the Contributing Guidelines on issues?
If the PR includes a change to an API's metadata (desc, label, params, etc.), does it also include updated API specs and example docs? — n/a

🤖 Generated with Claude Code

Sites map the `processing` status onto the `building` image, but the extension was still derived from the raw status, so a processing deployment asked for `status-building-light.png`, which does not exist. Only the `.gif` variants are shipped for `building`. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

greptile-apps · 2026-09-19T09:50:51Z

Confidence Score: 4/5

The production fix appears correct, but the implementation-coupled regression test must be revised to satisfy the repository’s explicit testing requirement before merging.

Findings

Test Mirrors Production Mapping ▶

Fix with agent prompt

### Issue 1
tests/unit/Vcs/CommentTest.php:140-146
This hard-coded status-to-filename table duplicates the mapping in `Comment::generateComment()` and pins the test to the current asset names and formats. That violates the repository directive to test observable behavior rather than mirror source code or configuration, and it means legitimate asset or mapping changes require synchronized test edits. This repository requirement must be satisfied before merging: inspect the image URLs generated for the relevant statuses-especially `processing`-and validate those emitted targets without independently prescribing their filenames.

Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!

---

For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.

Summary

This PR fixes Site deployment comments so the processing status references the existing animated building GIF rather than a nonexistent PNG.

Uses the mapped image status when selecting the extension, matching the existing Functions behavior.
Adds coverage for Site status image paths, but the test duplicates production filename mapping in violation of the repository’s testing rule.

_{Reviews (1) · Last reviewed commit: "fix(vcs): use the mapped status when pic..."}

greptile-apps · 2026-09-19T09:50:54Z

+        $extensions = [
+            'waiting' => 'status-waiting-%s.png',
+            'processing' => 'status-building-%s.gif',
+            'building' => 'status-building-%s.gif',
+            'ready' => 'status-ready-%s.png',
+            'failed' => 'status-failed-%s.png',
+        ];


Test Mirrors Production Mapping

This hard-coded status-to-filename table duplicates the mapping in Comment::generateComment() and pins the test to the current asset names and formats. That violates the repository directive to test observable behavior rather than mirror source code or configuration, and it means legitimate asset or mapping changes require synchronized test edits. This repository requirement must be satisfied before merging: inspect the image URLs generated for the relevant statuses—especially processing—and validate those emitted targets without independently prescribing their filenames.

Context Used: Call out and harshly judge implementation-coupled tests. We don't mirror source code, configuration, or version pins in assertions. We test observable behavior; use linters for syntax and schema checks. (source)

Prompt To Fix With AI

This is a comment left during a code review. Path: tests/unit/Vcs/CommentTest.php Line: 140-146 Comment: **Test Mirrors Production Mapping** This hard-coded status-to-filename table duplicates the mapping in `Comment::generateComment()` and pins the test to the current asset names and formats. That violates the repository directive to test observable behavior rather than mirror source code or configuration, and it means legitimate asset or mapping changes require synchronized test edits. This repository requirement must be satisfied before merging: inspect the image URLs generated for the relevant statuses—especially `processing`—and validate those emitted targets without independently prescribing their filenames. **Context Used:** Call out and harshly judge implementation-coupled tests. We don't mirror source code, configuration, or version pins in assertions. We test observable behavior; use linters for syntax and schema checks. ([source](https://app.greptile.com/review/custom-context?memory=instruction-0)) --- For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.

Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!

github-actions · 2026-09-19T09:55:42Z

✨ Benchmark results

Comparing main (before) → fix-vcs-comment-processing-image (after).

Metric	Before	After	Change
🚀 Requests/sec	`495.61`	`473.97`	⚪ `-4.4%`
⏱️ Latency P50	`34.94 ms`	`36.43 ms`	⚪ `+4.3%`
⏱️ Latency P95	`83.65 ms`	`86.89 ms`	⚪ `+3.9%`

Per-scenario breakdown & investigation details

Metrics below reflect the current branch (after). Δ P95 compares against the base.

Scenario	P50 (ms)	P95 (ms)	Requests	RPS	Δ P95 (ms)
API total	`36.43`	`86.89`	`28,899`	`473.97`	`+3.24`
Account	`63.34`	`124.45`	`1,521`	`25.33`	`+2.52`
TablesDB	`34.72`	`69.69`	`15,717`	`259.77`	`+4.59`
Storage	`33.88`	`83.89`	`7,605`	`127.5`	`+1.96`
Functions	`51.82`	`104.49`	`4,056`	`68.59`	`+3.33`

Top API waits (after)

API request	Max wait (ms)
storage.buckets.create	`257.26`
storage.files.create	`221.62`
account.name.update	`209.59`
functions.delete	`200.53`
storage.files.list	`182.19`

greptile-apps Bot reviewed Sep 19, 2026

View reviewed changes

Meldiron merged commit 05118a6 into main Sep 19, 2026
51 checks passed

Navigation Menu

fix(vcs): use the mapped status when picking the comment image extension - #13771

fix(vcs): use the mapped status when picking the comment image extension#13771
Meldiron merged 1 commit into
mainfrom
fix-vcs-comment-processing-image

Meldiron commented Sep 19, 2026 •

edited

Loading

Uh oh!

greptile-apps Bot commented Sep 19, 2026 •

edited

Loading

Uh oh!

greptile-apps Bot Sep 19, 2026

Uh oh!

github-actions Bot commented Sep 19, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Conversation

Meldiron commented Sep 19, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What does this PR do?

Test Plan

Related PRs and Issues

Checklist

Uh oh!

greptile-apps Bot commented Sep 19, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Confidence Score: 4/5

Findings

Summary

Uh oh!

greptile-apps Bot Sep 19, 2026

Choose a reason for hiding this comment

Uh oh!

github-actions Bot commented Sep 19, 2026

✨ Benchmark results

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Meldiron commented Sep 19, 2026 •

edited

Loading

greptile-apps Bot commented Sep 19, 2026 •

edited

Loading