|
| 1 | +# Comparison: PR #144 vs Commit d2f2e96 |
| 2 | + |
| 3 | +Both changes reorganize `.github/copilot-instructions.md` into AGENTS.md files, making the instructions |
| 4 | +universal for all AI agents rather than just GitHub Copilot. This document compares them point by point. |
| 5 | + |
| 6 | +## Overview of Each Change |
| 7 | + |
| 8 | +| | PR #144 | Commit d2f2e96 | |
| 9 | +|---|---|---| |
| 10 | +| **Title** | "Reorganize `.github/copilot-instructions.md` into `AGENTS.md` and `material3/AGENTS.md`" | "Simplify, transfer/reorganize `.github/copilot-instructions.md` into AGENTS.md and other more appropriate agent context files with Claude Code" | |
| 11 | +| **Author** | GitHub Copilot agent | ShreckYe (human, using Claude Code) | |
| 12 | +| **Status** | Open draft PR | Direct commit to a branch (not in main) | |
| 13 | +| **Files changed** | 3: delete `.github/copilot-instructions.md`, rewrite `AGENTS.md`, add `material3/AGENTS.md` | 3: delete `.github/copilot-instructions.md`, rewrite `AGENTS.md`, add `demo/AGENTS.md` | |
| 14 | +| **Net lines** | +282 / -407 | +162 / -407 | |
| 15 | + |
| 16 | +--- |
| 17 | + |
| 18 | +## Point-by-Point Comparison |
| 19 | + |
| 20 | +### 1. Path-Scoped File for Material 3 Lookup |
| 21 | + |
| 22 | +**PR #144**: Creates `material3/AGENTS.md` with a 7-line file containing the m3.material.io lookup |
| 23 | +guidance and a pointer to README.md. |
| 24 | + |
| 25 | +- **Pro**: Correctly path-scopes the M3 lookup guidance to the `material3/` subtree, so agents working |
| 26 | + in that directory receive relevant context automatically. |
| 27 | +- **Pro**: Clean separation of concerns — a contributor working only on `material3/` sees only what's |
| 28 | + relevant. |
| 29 | +- **Con**: Very thin file (7 lines); the full lookup context (e.g., "links to both Jetpack Compose and |
| 30 | + Material Web implementations") is present but brief. |
| 31 | + |
| 32 | +**Commit d2f2e96**: No separate `material3/AGENTS.md`. The M3 lookup guidance is merged into the "Adding |
| 33 | +New Components" section of root `AGENTS.md`. |
| 34 | + |
| 35 | +- **Con**: Not path-scoped; agents working in `material3/` do not get targeted context from a |
| 36 | + directory-level AGENTS.md. |
| 37 | +- **Pro**: Fewer files; the guidance is still present in the root for all agents. |
| 38 | + |
| 39 | +--- |
| 40 | + |
| 41 | +### 2. Path-Scoped File for Visual Validation (demo module) |
| 42 | + |
| 43 | +**PR #144**: Visual validation details are kept in the root `AGENTS.md` under the "Build and Test" |
| 44 | +section. |
| 45 | + |
| 46 | +- **Con**: Not path-scoped to the `demo/` directory; agents working in `demo/` don't get a dedicated |
| 47 | + context file. |
| 48 | + |
| 49 | +**Commit d2f2e96**: Extracts visual validation details into `demo/AGENTS.md` (20 lines). |
| 50 | + |
| 51 | +- **Pro**: Correctly path-scopes the browser automation / Playwright guidance to the `demo/` subtree. |
| 52 | +- **Pro**: The root `AGENTS.md` is less cluttered by low-level browser automation notes. |
| 53 | +- **Pro**: Mirrors the same path-scoping principle that PR #144 uses for `material3/`, but applied to the |
| 54 | + demo module instead. |
| 55 | + |
| 56 | +--- |
| 57 | + |
| 58 | +### 3. Overall Verbosity and Tone |
| 59 | + |
| 60 | +**PR #144**: More verbose, closer in style to the original `copilot-instructions.md`. Retains extensive |
| 61 | +explanations and rationale for each convention. |
| 62 | + |
| 63 | +- **Pro**: More instructive for human readers or less-capable AI agents that benefit from detailed |
| 64 | + context. |
| 65 | +- **Con**: ~282 added lines vs. 162 in the commit — larger maintenance surface. |
| 66 | +- **Con**: Some sections duplicate phrasing directly from the original without simplification. |
| 67 | + |
| 68 | +**Commit d2f2e96**: Significantly more concise. Uses compact prose, shorter bullet lists, and a |
| 69 | +table for the module structure. |
| 70 | + |
| 71 | +- **Pro**: Easier for AI agents to parse and act on; less noise. |
| 72 | +- **Pro**: Lower maintenance burden going forward. |
| 73 | +- **Con**: Some detail is lost (e.g., URL format for finding Compose UI source, full CSS shortcuts |
| 74 | + examples, full `apiDump` blockquote rationale). |
| 75 | + |
| 76 | +--- |
| 77 | + |
| 78 | +### 4. Module Structure Representation |
| 79 | + |
| 80 | +**PR #144**: Uses a directory-tree format (same as the original): |
| 81 | + |
| 82 | +``` |
| 83 | +common/ — Core APIs, layouts, modifiers |
| 84 | +material3/ — Material Design 3 components |
| 85 | +... |
| 86 | +``` |
| 87 | + |
| 88 | +**Commit d2f2e96**: Uses a Markdown table: |
| 89 | + |
| 90 | +| Module | Purpose | |
| 91 | +|--------|---------| |
| 92 | +| `common` | Core APIs, layouts, modifiers | |
| 93 | +| `material3` | Material Design 3 components | |
| 94 | + |
| 95 | +- **Commit pro**: Tables are easier for AI agents to parse; also notes `material2` is broken/unmaintained |
| 96 | + and `material-icons` uses the new `core`/`extended` split (which is more up-to-date). |
| 97 | +- **PR con**: Retains the `material-icons-core` name (old), not reflecting the updated split. |
| 98 | + |
| 99 | +--- |
| 100 | + |
| 101 | +### 5. Build Commands — API Check Failure Instructions |
| 102 | + |
| 103 | +**PR #144**: Includes the full `apiCheck` failure blockquote and three validation commands, but is |
| 104 | +**missing `wasmJsBrowserDevelopmentRun`**: |
| 105 | + |
| 106 | +```bash |
| 107 | +./gradlew publishToMavenLocal |
| 108 | +./gradlew :compose-multiplatform-html-unified-demo:run |
| 109 | +./gradlew :compose-multiplatform-html-unified-demo:jsBrowserDevelopmentRun # for JS browser |
| 110 | +``` |
| 111 | + |
| 112 | +- **Con**: Omits the `wasmJsBrowserDevelopmentRun` validation command that is important for Compose UI |
| 113 | + rendering verification. |
| 114 | + |
| 115 | +**Commit d2f2e96**: Compact single-sentence policy + all validation commands are listed in the "Demo |
| 116 | +Validation" block (including `wasmJsBrowserDevelopmentRun`). The failure note refers agents to use |
| 117 | +`publishToMavenLocal` and demo runs. |
| 118 | + |
| 119 | +- **Pro**: `wasmJsBrowserDevelopmentRun` is present in the demo validation list. |
| 120 | +- **Con**: The specific "if `apiCheck` fails, do these" commands are not repeated in a dedicated block, |
| 121 | + so agents must infer they should use the demo validation commands. |
| 122 | + |
| 123 | +--- |
| 124 | + |
| 125 | +### 6. Demo Validation Commands |
| 126 | + |
| 127 | +**PR #144**: Lists all four demo commands under "Demo Validation" including `sideBySideBrowserDistribution`. |
| 128 | + |
| 129 | +**Commit d2f2e96**: Lists all four demo commands as well. |
| 130 | + |
| 131 | +Both are equivalent here. ✓ |
| 132 | + |
| 133 | +--- |
| 134 | + |
| 135 | +### 7. Troubleshooting / Build Issues |
| 136 | + |
| 137 | +**PR #144**: Has a "Troubleshooting" section with four bullets (Network, Memory/OOM, kotlin-js-store, |
| 138 | +Gradle daemon timeout). Explicitly notes `--no-daemon` flag. |
| 139 | + |
| 140 | +**Commit d2f2e96**: Has the same four bullets, slightly more compact. The `kotlin-js-store` note is |
| 141 | +placed prominently as a standalone "Note" in the Build Commands section. |
| 142 | + |
| 143 | +Both are equivalent in content; the commit surfaces the `kotlin-js-store` note more prominently. ✓ |
| 144 | + |
| 145 | +--- |
| 146 | + |
| 147 | +### 8. Snapshot Dependencies Note |
| 148 | + |
| 149 | +**PR #144**: Placed in a "Prerequisites" section with an **IMPORTANT** callout at the top of the Build |
| 150 | +section. |
| 151 | + |
| 152 | +**Commit d2f2e96**: A short plain sentence at the end of the Build Commands section. |
| 153 | + |
| 154 | +- **PR pro**: More visible; agents are less likely to miss it. |
| 155 | +- **Commit con**: Easy to overlook since it appears after the main commands. |
| 156 | + |
| 157 | +--- |
| 158 | + |
| 159 | +### 9. Code Style — Trailing Commas |
| 160 | + |
| 161 | +**PR #144**: Full bullet list with all exceptions and a code example showing correct vs. incorrect |
| 162 | +placement with inline comments. |
| 163 | + |
| 164 | +**Commit d2f2e96**: Single-sentence summary ("Add to the last parameter of multi-line Composable |
| 165 | +parameter/argument lists. Do NOT add for single-line lists, single parameters, or annotations."). |
| 166 | + |
| 167 | +- **PR pro**: All edge cases are explicit (e.g., annotation exception, end-of-line comment convention). |
| 168 | +- **Commit con**: The end-of-line comment placement rule is not mentioned at all. |
| 169 | + |
| 170 | +--- |
| 171 | + |
| 172 | +### 10. Code Style — Nullable Composable Types |
| 173 | + |
| 174 | +**PR #144**: Multi-bullet explanation with the "do not use" counterexample and the extension function |
| 175 | +receiver exception. |
| 176 | + |
| 177 | +**Commit d2f2e96**: Single line: `Use @Composable (() -> Unit)?`, no mention of the extension receiver |
| 178 | +exception. |
| 179 | + |
| 180 | +- **PR pro**: Complete — includes the important `fun (@Composable …).foo()` exception. |
| 181 | +- **Commit con**: The extension function receiver exception is missing. |
| 182 | + |
| 183 | +--- |
| 184 | + |
| 185 | +### 11. Expect/Actual Patterns |
| 186 | + |
| 187 | +**PR #144**: Three detailed bullets for `expect interface`, `expect enum class`, and `expect class`, with |
| 188 | +concrete type examples (`SnackbarVisuals`, `SnackbarHostState`). |
| 189 | + |
| 190 | +**Commit d2f2e96**: Same three patterns but compressed to 3 bullet lines with no specific type examples. |
| 191 | + |
| 192 | +- **PR pro**: Easier to understand from the concrete examples. |
| 193 | +- **Commit pro**: More scannable for an agent that only needs to know the rule. |
| 194 | + |
| 195 | +--- |
| 196 | + |
| 197 | +### 12. Component Organization (main vs. ext package) |
| 198 | + |
| 199 | +**PR #144**: Two numbered items with sub-bullets and examples; labs annotation as a third item. |
| 200 | + |
| 201 | +**Commit d2f2e96**: Two bullets (main/ext), then labs deps mentioned in "Adding New Components" step 5. |
| 202 | + |
| 203 | +- **PR pro**: Labs annotations clearly documented as a third organization principle. |
| 204 | +- **Commit con**: Labs annotation treatment is present but spread across two sections. |
| 205 | + |
| 206 | +--- |
| 207 | + |
| 208 | +### 13. Unsupported Compose UI APIs (commented-out lines pattern) |
| 209 | + |
| 210 | +**PR #144**: Full KDoc example with two `@param` bullets and the complete `expect fun Text(...)` showing |
| 211 | +two commented-out parameters. |
| 212 | + |
| 213 | +**Commit d2f2e96**: Shorter example with one `@param` bullet and one commented-out parameter. |
| 214 | + |
| 215 | +- **PR pro**: The two-parameter example shows the pattern more clearly as a list. |
| 216 | +- Both are functionally equivalent. The detail difference is minor. |
| 217 | + |
| 218 | +--- |
| 219 | + |
| 220 | +### 14. Platform-Limited Parameters |
| 221 | + |
| 222 | +**PR #144**: Dedicated subsection with two bullets and two concrete examples (`enabled` in tabs, `space` |
| 223 | +in segmented button rows). |
| 224 | + |
| 225 | +**Commit d2f2e96**: Same rule as a short paragraph; no concrete examples. |
| 226 | + |
| 227 | +- **PR pro**: Concrete examples make the rule immediately actionable. |
| 228 | + |
| 229 | +--- |
| 230 | + |
| 231 | +### 15. "Copied and Adapted" Code |
| 232 | + |
| 233 | +**PR #144**: Includes the full URL pattern for finding Compose Multiplatform Core source: |
| 234 | +``` |
| 235 | +https://raw.githubusercontent.com/JetBrains/compose-multiplatform-core/v<VERSION>/... |
| 236 | +``` |
| 237 | +Also includes the three "ensure" checklist items and the exception note. |
| 238 | + |
| 239 | +**Commit d2f2e96**: Shorter. Only gives the comment format, omits the URL pattern for finding source |
| 240 | +and the three checklist items. |
| 241 | + |
| 242 | +- **PR pro**: The URL pattern is highly useful when agents need to actually find the original source. |
| 243 | +- **PR pro**: The three checklist items (names/types, definition order, unsupported APIs documented) are |
| 244 | + a useful quality gate. |
| 245 | +- **Commit con**: Agents may not know how to find the upstream Compose UI source. |
| 246 | + |
| 247 | +--- |
| 248 | + |
| 249 | +### 16. CSS Style Shortcuts |
| 250 | + |
| 251 | +**PR #144**: Dedicated "CSS Style Shortcuts" subsection with two `overflow()`/`minHeight()` examples |
| 252 | +and a note about upstream contribution. |
| 253 | + |
| 254 | +**Commit d2f2e96**: One-line bullet in "JS DOM Notes": "Prefer type-safe CSS shortcuts over raw |
| 255 | +`property()` calls". |
| 256 | + |
| 257 | +- **PR pro**: The concrete examples (`overflow(Overflow.Hidden)` vs `property("overflow", "hidden")`) |
| 258 | + make the rule instantly clear. |
| 259 | +- **Commit con**: No examples; agents may not know which shortcuts exist or how to use them. |
| 260 | + |
| 261 | +--- |
| 262 | + |
| 263 | +### 17. `fillMax*Stretch` vs `fillMax*` |
| 264 | + |
| 265 | +**PR #144**: Full dedicated subsection with the full explanation of why `100%` causes overflow and when |
| 266 | +to use each variant. |
| 267 | + |
| 268 | +**Commit d2f2e96**: One-line bullet in "JS DOM Notes": "Prefer `fillMaxWidthStretch()` over |
| 269 | +`fillMaxWidth()` — avoids overflow from padding/margin (uses CSS `stretch` instead of `100%`)". |
| 270 | + |
| 271 | +- **PR pro**: More complete explanation; agents understand when NOT to use the stretch variants (when |
| 272 | + `fraction` parameter is needed). |
| 273 | +- **Commit con**: The `fraction` parameter exception is not mentioned. |
| 274 | + |
| 275 | +--- |
| 276 | + |
| 277 | +### 18. Avoiding Duplicate Components |
| 278 | + |
| 279 | +**PR #144**: Dedicated "Avoiding Duplicate Components" subsection with a concrete `ListScope.ListItem` |
| 280 | +example. |
| 281 | + |
| 282 | +**Commit d2f2e96**: One-line bullet in "JS DOM Notes": "Search existing codebase before adding components |
| 283 | +to avoid duplicates". |
| 284 | + |
| 285 | +- **PR pro**: The concrete example is a useful reminder. |
| 286 | + |
| 287 | +--- |
| 288 | + |
| 289 | +### 19. Adding New Components — Step List |
| 290 | + |
| 291 | +**PR #144**: Four steps: |
| 292 | +1. Add to demo (`Material3.kt`) |
| 293 | +2. Visual consistency comparison (side-by-side demo) |
| 294 | +3. Note on inherent platform differences |
| 295 | +4. README update |
| 296 | + |
| 297 | +**Commit d2f2e96**: Five steps: |
| 298 | +1. **Implement** in `commonMain`, `composeUiMain`, `jsMain` |
| 299 | +2. Add to demo |
| 300 | +3. Compare rendering via side-by-side demo (links to `demo/AGENTS.md` for browser automation details) |
| 301 | +4. Update README.md API catalog |
| 302 | +5. Mark JS labs dependencies with `@MaterialWebLabsApi` |
| 303 | + |
| 304 | +- **Commit pro**: Step 1 (implement) is explicit — the PR assumes agents know to implement before adding |
| 305 | + to demo. |
| 306 | +- **Commit pro**: Step 5 (labs deps) is explicit here; in PR it's in the component organization section. |
| 307 | +- **PR pro**: Step 3 (inherent platform differences note) reminds agents to document them. |
| 308 | +- **Commit pro**: Cross-references `demo/AGENTS.md` for browser automation details, demonstrating |
| 309 | + intentional path-scoping design. |
| 310 | + |
| 311 | +--- |
| 312 | + |
| 313 | +### 20. Sections Removed vs. Original (both agree) |
| 314 | + |
| 315 | +Both approaches agree on removing the following from the original `copilot-instructions.md` (good |
| 316 | +decision in both cases): |
| 317 | + |
| 318 | +- Repository statistics (size, language — already obvious) |
| 319 | +- Hardcoded dependency version numbers (AGP 8.12.3, Kotlin 2.3.20, etc.) — become stale quickly |
| 320 | +- Build timing estimates (5–10 min, 1–3 min, etc.) — non-actionable |
| 321 | +- Root directory file list — low value |
| 322 | +- "Trust These Instructions" meta-notes — not actionable for agents |
| 323 | +- `./gradlew --version` basic setup check |
| 324 | +- `--continue` flag note for platform-specific builds |
| 325 | + |
| 326 | +--- |
| 327 | + |
| 328 | +### 21. Section Present in Neither (potential gap) |
| 329 | + |
| 330 | +The following item from the original `copilot-instructions.md` is **not present in either** change: |
| 331 | + |
| 332 | +- **CSS imports for Material Icons** ("Located in `jsMain` source sets — requires CSS imports for Material |
| 333 | + Icons"): Neither approach preserves this note, which could cause agents to miss a required step when |
| 334 | + adding icon support. |
| 335 | + |
| 336 | +--- |
| 337 | + |
| 338 | +## Summary Table |
| 339 | + |
| 340 | +| Point | PR #144 | Commit d2f2e96 | |
| 341 | +|---|---|---| |
| 342 | +| `material3/AGENTS.md` (path-scoped) | ✅ Created | ❌ Not created | |
| 343 | +| `demo/AGENTS.md` (path-scoped) | ❌ Not created | ✅ Created | |
| 344 | +| Overall verbosity | Verbose (more detail) | Concise (AI-friendly) | |
| 345 | +| Module structure representation | Directory tree | Markdown table (more scannable) | |
| 346 | +| `wasmJsBrowserDevelopmentRun` in apiCheck instructions | ❌ Missing | ✅ Present | |
| 347 | +| Snapshot dependencies visibility | ✅ IMPORTANT callout | ⚠️ Easy to overlook | |
| 348 | +| Trailing comma: end-of-line comment rule | ✅ Explicit | ❌ Missing | |
| 349 | +| Nullable composable: extension receiver exception | ✅ Present | ❌ Missing | |
| 350 | +| Expect/actual: concrete type examples | ✅ Yes | ❌ No | |
| 351 | +| Labs annotation as organization principle | ✅ Explicit section | ⚠️ Split across sections | |
| 352 | +| Unsupported APIs: full KDoc example | ✅ Two params | ⚠️ One param | |
| 353 | +| Platform-limited params: concrete examples | ✅ Yes | ❌ No | |
| 354 | +| "Copied and adapted": source URL pattern | ✅ Yes | ❌ Missing | |
| 355 | +| CSS shortcuts: concrete examples | ✅ Yes | ❌ No | |
| 356 | +| `fillMax*Stretch`: fraction exception | ✅ Explicit | ❌ Missing | |
| 357 | +| Avoiding duplicates: concrete example | ✅ Yes | ❌ No | |
| 358 | +| Adding components: "implement" step | ❌ Missing | ✅ Explicit | |
| 359 | +| Adding components: labs deps step | ⚠️ In org. section | ✅ Explicit step | |
| 360 | +| Adding components: inherent differences note | ✅ Step 3 | ❌ Missing | |
| 361 | +| CSS imports for Material Icons note | ❌ Missing | ❌ Missing | |
| 362 | + |
| 363 | +--- |
| 364 | + |
| 365 | +## Recommendation |
| 366 | + |
| 367 | +The two approaches are complementary. An ideal merged result would: |
| 368 | + |
| 369 | +1. **Adopt the commit's path-scoping strategy for both** `demo/AGENTS.md` AND `material3/AGENTS.md`. |
| 370 | +2. **Adopt the commit's conciseness** for the overall AGENTS.md to reduce maintenance burden. |
| 371 | +3. **Restore from PR #144**: |
| 372 | + - The end-of-line comment trailing comma rule |
| 373 | + - The extension function receiver exception for nullable composable types |
| 374 | + - The source URL pattern in "Copied and Adapted" |
| 375 | + - The `fraction` parameter exception for `fillMax*Stretch` |
| 376 | + - Concrete CSS shortcut examples (at least one) |
| 377 | +4. **Adopt from commit d2f2e96**: |
| 378 | + - The "implement" step (step 1) in Adding New Components |
| 379 | + - The labs deps step in Adding New Components |
| 380 | + - The table representation for module structure |
| 381 | + - The explicit `wasmJsBrowserDevelopmentRun` in apiCheck failure instructions |
| 382 | +5. **Add to both** (currently missing from both): CSS imports for Material Icons note. |
0 commit comments