Use property descriptions for completion items (#283)

* Use property descriptions for completion items

* Add test for type description fallback

PR_REVIEW.md

@@ -0,0 +1,59 @@
+# PR #283 Review: Use property descriptions for completion items
+
+## Summary
+
+This PR fixes a bug where completion items for action.yml were missing descriptions. The root cause was that `mappingValues()` only looked at the type definition's description, ignoring property-level descriptions in the schema.
+
+## Changes Analysis
+
+### Core Fix ([definition.ts](languageservice/src/value-providers/definition.ts))
+
+**Before:**
+```typescript
+let description: string | undefined;
+if (value.type) {
+  const typeDef = definitions[value.type];
+  description = typeDef?.description;
+}
+```
+
+**After:**
+```typescript
+let description: string | undefined = value.description;
+if (value.type) {
+  const typeDef = definitions[value.type];
+  if (!description) {
+    description = typeDef?.description;
+  }
+}
+```
+
+✅ **Correct approach** - prioritizes property description, falls back to type description.
+
+### Test Coverage
+
+1. **complete-action.test.ts**: Two new tests verify `author` and `branding` completions include documentation
+2. **hover-action.test.ts**: New test for `author` hover + updated `branding` test to verify "Documentation" link
+
+## Potential Issues
+
+### 1. One-of expansion doesn't use property description
+
+Looking at line 140-142:
+```typescript
+const expanded = expandOneOfToCompletions(oneOfDef, definitions, key, description, indentation, mode);
+```
+
+This passes `description` to `expandOneOfToCompletions`, but at this point `description` may have been populated from the property. **This is correct** - the property description is passed through.
+
+### 2. Consistency check
+
+The PR description mentions this is consistent with hover. Verified: [template-reader.ts#L225](workflow-parser/src/templates/template-reader.ts#L225) shows hover uses `nextPropertyDef.description` when available.
+
+## Verdict
+
+✅ **LGTM** - Clean, minimal fix that aligns completion behavior with hover. Good test coverage for the specific cases mentioned.
+
+## Minor Suggestions (non-blocking)
+
+1. Could add a test for a property that has NO description but whose type DOES have one, to verify fallback works (e.g., `inputs` which references `inputs-strict` type that has a description)

languageservice/src/complete-action.test.ts

@@ -251,6 +251,38 @@ runs:
       expect(labels).not.toContain("jobs");
     });
 
+    it("includes descriptions from schema for completion items", async () => {
+      const [doc, position] = createActionDocument(`|`, "file:///my-repo/action.yml");
+      const completions = await complete(doc, position);
+
+      const authorCompletion = completions.find(c => c.label === "author");
+      expect(authorCompletion).toBeDefined();
+      expect(authorCompletion?.documentation).toBeDefined();
+      expect((authorCompletion?.documentation as {value: string})?.value).toContain("author");
+    });
+
+    it("includes descriptions for branding completion", async () => {
+      const [doc, position] = createActionDocument(`|`, "file:///my-repo/action.yml");
+      const completions = await complete(doc, position);
+
+      const brandingCompletion = completions.find(c => c.label === "branding");
+      expect(brandingCompletion).toBeDefined();
+      expect(brandingCompletion?.documentation).toBeDefined();
+      expect((brandingCompletion?.documentation as {value: string})?.value).toContain("branding");
+    });
+
+    it("falls back to type description when property has no description", async () => {
+      // `inputs` uses shorthand form in schema: "inputs": "inputs-strict"
+      // So the property has no description, but the type `inputs-strict` does
+      const [doc, position] = createActionDocument(`|`, "file:///my-repo/action.yml");
+      const completions = await complete(doc, position);
+
+      const inputsCompletion = completions.find(c => c.label === "inputs");
+      expect(inputsCompletion).toBeDefined();
+      expect(inputsCompletion?.documentation).toBeDefined();
+      expect((inputsCompletion?.documentation as {value: string})?.value).toContain("Input parameters");
+    });
+
     it("does not route workflow files to action completion", async () => {
       const doc = TextDocument.create("file:///repo/.github/workflows/ci.yml", "yaml", 1, `o`);
       const completions = await complete(doc, {line: 0, character: 1});

languageservice/src/hover-action.test.ts

@@ -53,6 +53,20 @@ ru|ns:
       expect(result).not.toBeNull();
       expect(result?.contents).toContain("runs");
     });
+
+    it("shows description for author key", async () => {
+      const [doc, position] = createActionDocument(`name: My Action
+description: Test
+au|thor: Me
+runs:
+  using: node20
+  main: index.js`);
+      const result = await hover(doc, position);
+
+      expect(result).not.toBeNull();
+      expect(result?.contents).toContain("author");
+      expect(result?.contents).toContain("Documentation");
+    });
   });
 
   describe("runs properties", () => {
@@ -145,6 +159,7 @@ brand|ing:
 
       expect(result).not.toBeNull();
       expect(result?.contents).toContain("brand");
+      expect(result?.contents).toContain("Documentation");
     });
 
     it("shows description for icon key", async () => {

languageservice/src/value-providers/definition.ts

@@ -107,10 +107,14 @@ function mappingValues(
   for (const [key, value] of Object.entries(mappingDefinition.properties)) {
     let insertText: string | undefined;
 
-    let description: string | undefined;
+    // Prefer the property's own description (from the schema's property definition),
+    // fall back to the type definition's description if the property doesn't have one
+    let description: string | undefined = value.description;
     if (value.type) {
       const typeDef = definitions[value.type];
-      description = typeDef?.description;
+      if (!description) {
+        description = typeDef?.description;
+      }
 
       if (typeDef) {
         switch (typeDef.definitionType) {