Adds a new MCP resource endpoint to read revisions from Phabricator

[suhaibmujahid]

Resolves #5302

[Copilot]

Pull Request Overview

This PR adds a new MCP (Model Context Protocol) resource endpoint to read revisions from Phabricator, allowing users to retrieve comprehensive revision information including metadata, diffs, and comments in markdown format.

Key changes:

• Adds a new MCP resource handler for Phabricator revisions
• Refactors the PhabricatorPatch class to support multiple initialization methods
• Introduces new classes for handling Phabricator comments with proper type hierarchies

Reviewed Changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 4 comments.

File	Description
mcp/src/bugbug_mcp/server.py	Adds new MCP resource endpoint for Phabricator revisions
bugbug/tools/code_review.py	Major refactoring of PhabricatorPatch class and addition of comment handling classes

---

_{Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.}

[suhaibmujahid]

Note

This is an example of the content when reading a Phabricator revision from the MCP server

Revision D267032: Bug 1991955- Add skeleton loader animation to show while images load. r=gl

Basic Information

• URI: https://phabricator.services.mozilla.com/D267032
• Revision Author: PHID-USER-6sv6tp4gujvlfycgrput
• Title: Bug 1991955- Add skeleton loader animation to show while images load. r=gl
• Status: Needs Review
• Created: 2025-10-01 12:12:46
• Modified: 2025-10-01 12:12:47
• Bugzilla Bug: 1991955

Diff Information

• Diff ID: 1131598
• Base Revision: ad22dc970588

Stack Information

Dependency Graph:

graph TD
    D267032[Bug 1991955- Add skeleton loader animation to show while images load. r=gl - CURRENT]
    style D267032 fill:#105823
    D267032 --> D265168
    D265168[Bug 1988271- Update number of columns on the Stories Discover More page to display based on different window size classes]
    D265168 --> D265167
    D265167[Bug 1988204- Update Stories Discover More screen to match figma.]

Loading

Code Changes

diff --git a/mobile/android/fenix/app/src/main/java/org/mozilla/fenix/home/pocket/ui/StoryCard.kt b/mobile/android/fenix/app/src/main/java/org/mozilla/fenix/home/pocket/ui/StoryCard.kt
--- a/mobile/android/fenix/app/src/main/java/org/mozilla/fenix/home/pocket/ui/StoryCard.kt
+++ b/mobile/android/fenix/app/src/main/java/org/mozilla/fenix/home/pocket/ui/StoryCard.kt
@@ -3,15 +3,16 @@
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 
 package org.mozilla.fenix.home.pocket.ui
 
 import androidx.compose.foundation.layout.Arrangement
+import androidx.compose.foundation.layout.Box
 import androidx.compose.foundation.layout.Column
 import androidx.compose.foundation.layout.Row
 import androidx.compose.foundation.layout.Spacer
+import androidx.compose.foundation.layout.aspectRatio
 import androidx.compose.foundation.layout.fillMaxWidth
-import androidx.compose.foundation.layout.height
 import androidx.compose.foundation.layout.padding
 import androidx.compose.foundation.layout.width
 import androidx.compose.foundation.shape.RoundedCornerShape
 import androidx.compose.material3.Card
 import androidx.compose.material3.CardDefaults
@@ -31,26 +32,36 @@
 import mozilla.components.service.pocket.PocketStory.PocketRecommendedStory
 import mozilla.components.service.pocket.PocketStory.PocketSponsoredStory
 import mozilla.components.service.pocket.PocketStory.SponsoredContent
 import org.mozilla.fenix.compose.Favicon
 import org.mozilla.fenix.compose.Image
+import org.mozilla.fenix.compose.skeletonLoader
 import org.mozilla.fenix.home.fake.FakeHomepagePreview
 import org.mozilla.fenix.theme.FirefoxTheme
 import kotlin.math.roundToInt
 
 private val cardShape = RoundedCornerShape(8.dp)
 private val defaultCardContentPadding = 8.dp
-private val imageSize = 345.dp
+private val imageWidth = 345.dp
 private val imageHeight = 180.dp
 
 @OptIn(ExperimentalMaterial3Api::class)
 @Composable
 internal fun StoryCard(
     story: PocketStory,
     onClick: (story: PocketStory, position: Triple<Int, Int, Int>) -> Unit,
     modifier: Modifier = Modifier,
 ) {
+    val imageUrl = story.imageUrl.replace(
+        "{wh}",
+        with(LocalDensity.current) {
+            "${imageWidth.toPx().roundToInt()}x${
+                imageWidth.toPx().roundToInt()
+            }"
+        },
+    )
+
     Card(
         onClick = {
             onClick(story, Triple(0, 0, 0))
         },
         modifier = modifier,
@@ -59,38 +70,33 @@
         colors = CardDefaults.cardColors(containerColor = FirefoxTheme.colors.layer2),
     ) {
         Column(
             modifier = Modifier.padding(all = defaultCardContentPadding),
         ) {
-            val imageUrl = story.imageUrl.replace(
-                "{wh}",
-                with(LocalDensity.current) {
-                    "${imageSize.toPx().roundToInt()}x${
-                        imageSize.toPx().roundToInt()
-                    }"
-                },
-            )
-
             Image(
                 url = imageUrl,
                 modifier = Modifier
                     .fillMaxWidth()
-                    .height(imageHeight)
+                    .aspectRatio(imageWidth / imageHeight)
                     .clip(cardShape),
                 private = false,
-                targetSize = imageSize,
+                targetSize = imageWidth,
                 contentScale = ContentScale.Crop,
+                placeholder = {
+                    Placeholder()
+                },
             )
 
             Column(
                 modifier = Modifier.padding(all = 8.dp),
                 verticalArrangement = Arrangement.spacedBy(8.dp),
             ) {
                 Text(
                     text = story.title,
                     color = FirefoxTheme.colors.textPrimary,
                     overflow = TextOverflow.Ellipsis,
+                    maxLines = 2,
                     style = FirefoxTheme.typography.headline7,
                 )
 
                 var subtitle = ""
                 when (story) {
@@ -102,11 +108,11 @@
                         subtitle = story.sponsor
                     }
 
                     is PocketRecommendedStory,
                     is PocketSponsoredStory,
-                    -> {
+                        -> {
                         // no-op, don't handle these [PocketStory] types as they are no longer
                         // supported after the Merino recommendation migration.
                     }
                 }
 
@@ -127,10 +133,19 @@
             }
         }
     }
 }
 
+@Composable
+private fun Placeholder() {
+    Box(
+        modifier = Modifier
+            .aspectRatio(imageWidth / imageHeight)
+            .skeletonLoader(),
+    )
+}
+
 @PreviewLightDark
 @Composable
 private fun StoryCardPreview() {
     FirefoxTheme {
         Column(modifier = Modifier.padding(16.dp)) {
diff --git a/mobile/android/fenix/app/src/main/java/org/mozilla/fenix/home/pocket/ui/StoriesScreen.kt b/mobile/android/fenix/app/src/main/java/org/mozilla/fenix/home/pocket/ui/StoriesScreen.kt
--- a/mobile/android/fenix/app/src/main/java/org/mozilla/fenix/home/pocket/ui/StoriesScreen.kt
+++ b/mobile/android/fenix/app/src/main/java/org/mozilla/fenix/home/pocket/ui/StoriesScreen.kt
@@ -142,11 +142,10 @@
         horizontalArrangement = Arrangement.spacedBy(16.dp, alignment = Alignment.CenterHorizontally),
     ) {
         state.pocketStories.forEachIndexed { index, story ->
             item {
                 StoryCard(
-                    modifier = Modifier.padding(horizontal = 16.dp),
                     story = story,
                     onClick = interactor::onStoryClicked,
                 )
             }
         }
diff --git a/mobile/android/fenix/app/src/main/java/org/mozilla/fenix/compose/ListItemTabSurface.kt b/mobile/android/fenix/app/src/main/java/org/mozilla/fenix/compose/ListItemTabSurface.kt
--- a/mobile/android/fenix/app/src/main/java/org/mozilla/fenix/compose/ListItemTabSurface.kt
+++ b/mobile/android/fenix/app/src/main/java/org/mozilla/fenix/compose/ListItemTabSurface.kt
@@ -2,13 +2,20 @@
  * License, v. 2.0. If a copy of the MPL was not distributed with this
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 
 package org.mozilla.fenix.compose
 
+import androidx.compose.animation.animateColor
+import androidx.compose.animation.core.LinearEasing
+import androidx.compose.animation.core.RepeatMode
+import androidx.compose.animation.core.infiniteRepeatable
+import androidx.compose.animation.core.rememberInfiniteTransition
+import androidx.compose.animation.core.tween
 import androidx.compose.foundation.BorderStroke
 import androidx.compose.foundation.clickable
 import androidx.compose.foundation.layout.Arrangement
+import androidx.compose.foundation.layout.Box
 import androidx.compose.foundation.layout.Column
 import androidx.compose.foundation.layout.ColumnScope
 import androidx.compose.foundation.layout.PaddingValues
 import androidx.compose.foundation.layout.Row
 import androidx.compose.foundation.layout.Spacer
@@ -20,13 +27,15 @@
 import androidx.compose.material3.Card
 import androidx.compose.material3.CardDefaults
 import androidx.compose.material3.MaterialTheme
 import androidx.compose.material3.Text
 import androidx.compose.runtime.Composable
+import androidx.compose.runtime.getValue
 import androidx.compose.ui.Alignment
 import androidx.compose.ui.Modifier
 import androidx.compose.ui.draw.clip
+import androidx.compose.ui.draw.drawBehind
 import androidx.compose.ui.graphics.Color
 import androidx.compose.ui.layout.ContentScale
 import androidx.compose.ui.tooling.preview.PreviewLightDark
 import androidx.compose.ui.unit.dp
 import androidx.compose.ui.unit.sp
@@ -84,10 +93,13 @@
                 url = imageUrl,
                 modifier = imageModifier,
                 private = false,
                 targetSize = imageWidth,
                 contentScale = imageContentScale,
+                placeholder = {
+                    Box(modifier = Modifier.size(IMAGE_SIZE.dp).skeletonLoader())
+                },
             )
 
             Spacer(Modifier.width(FirefoxTheme.layout.space.static100))
 
             Column(
@@ -129,5 +141,40 @@
                 fontSize = 14.sp,
             )
         }
     }
 }
+
+/**
+ * Applies a shimmering skeleton loading effect to the current [Modifier].
+ *
+ * This can be used as a placeholder for UI elements while their content is loading.
+ *
+ * @param durationMillis The duration in milliseconds of the shimmer animation cycle.
+ * Defaults to `1000`.
+ * @param color1 The starting color of the gradient animation. Defaults to [Color.LightGray].
+ * @param color2 The ending color of the gradient animation. Defaults to [Color.White].
+ *
+ * @return A [Modifier] that displays a skeleton loader effect.
+ */
+@Composable
+fun Modifier.skeletonLoader(
+    durationMillis: Int = 1000,
+    color1: Color = Color.LightGray,
+    color2: Color = Color.White,
+): Modifier {
+    val transition = rememberInfiniteTransition(label = "")
+
+    val color by transition.animateColor(
+        initialValue = color1,
+        targetValue = color2,
+        animationSpec = infiniteRepeatable(
+            animation = tween(durationMillis, easing = LinearEasing),
+            repeatMode = RepeatMode.Reverse,
+        ),
+        label = "",
+    )
+
+    return drawBehind {
+        drawRect(color = color)
+    }
+}

Comments Timeline

2025-10-01 16:28:57 - General Comment by PHID-USER-6sv6tp4gujvlfycgrput

I'm looking for feedback on the name of the modifier extension function, as well as the location of that extension function.

---

2025-10-01 16:30:15 - Inline Comment by PHID-USER-6sv6tp4gujvlfycgrput on mobile/android/fenix/app/src/main/java/org/mozilla/fenix/compose/ListItemTabSurface.kt at Line 165 [RESOLVED]

should there be anything here for the label?

---

2025-10-01 16:30:26 - Inline Comment by PHID-USER-6sv6tp4gujvlfycgrput on mobile/android/fenix/app/src/main/java/org/mozilla/fenix/compose/ListItemTabSurface.kt at Line 174 [RESOLVED]

Same here

---

[marco-c]

Another thing we could add is the linting results from review bot, but let's add it as a follow-up

[marco-c]

Feel free to land after addressing the comments