fix(ego-lint): reduce Just Perfection false positives

Field test #10 on Just Perfection (most popular GNOME extension) revealed
6 false positives and a duplicate-detection bug. Fixes:

- License check: add src/ parent-dir fallback (FAIL → PASS)
- UUID-dir match: skip when dirname is "src" (src/ layout)
- isLocked guard: sessionMode.isLocked is always a guard pattern, not
  lock screen functionality — exempt from impossible-state and
  session-modes-consistency checks
- Prototype-override: deduplicate by (file, prototype) to avoid
  counting both override and restore
- Comment-density: raise threshold from 40% to 50% to accommodate
  well-documented hand-written code (JSDoc-heavy)

Results: FAILs 3→2, WARNs 414→408 (remaining 400 R-SLOP addressed by PR #23)

Closes #26, closes #27, closes #28

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

Author: ZviBaratz

docs/internal/field-test-just-perfection.md

@@ -0,0 +1,121 @@
+# Field Test #10: Just Perfection
+
+## Pre-flight
+
+- **Extension**: Just Perfection (`just-perfection-desktop@just-perfection`)
+- **Source**: https://github.com/jrahmatzadeh/just-perfection
+- **GNOME versions**: 45, 46, 47, 48, 49, 50
+- **File count**: 7 JS files, 2 CSS files; ~5,300 JS lines (Manager.js alone is 1,636 non-blank)
+- **License**: GPL-3.0-only (in repo root, not in `src/`)
+- **Why this extension**: Most popular GNOME extension; exercises connectObject/disconnectObject signal management (67 signals), prototype backup/restore via `#originals` map, `src/` directory layout, GNOME 50 support (widest version range), CSS-heavy (23.7%), private field usage (`#timeoutIds`, `#originals`, `#shellVersion`), 20 Shell.ui imports
+
+## Step 1: ego-lint
+
+```bash
+./ego-lint --verbose /tmp/just-perfection/src
+```
+
+### Results (before fixes)
+
+| Status | Count |
+|--------|-------|
+| PASS   | 202   |
+| FAIL   | 3     |
+| WARN   | 414   |
+| SKIP   | 12    |
+| Exit   | 1     |
+
+### Classification of Each FAIL
+
+| Rule | File:Line | TP/FP | Root Cause (if FP) |
+|------|-----------|-------|-------------------|
+| `license` | — | **FP** | LICENSE in repo root, not in `src/`; no parent-dir fallback for src/ layout |
+| `R-VER48-07` | stylesheet.css:167 | TP | `.quick-menu-toggle` renamed to `.quick-toggle-has-menu` in GNOME 48 |
+| `metadata/future-shell-version` | — | TP | GNOME 50 > CURRENT_STABLE (49) |
+
+### Classification of Key WARNs
+
+| Rule | Count | Classification | Notes |
+|------|-------|---------------|-------|
+| R-SLOP-01 (JSDoc @param) | 129 | **FP** | Provenance score=3 but threshold was >=4; PR #23 fixes threshold to >=3 |
+| R-SLOP-02 (JSDoc @returns) | 271 | **FP** | Same root cause as R-SLOP-01 |
+| `lifecycle/prototype-override` | 5 (3 unique) | TP + **bug** | Advisory is valid (prototypes restored via close() chain). Regex catches both override AND restore → duplicates |
+| `metadata/uuid-matches-dir` | 1 | **FP** | Dir is `src`, not UUID; meaningless for src/ layout |
+| `metadata/session-modes-consistency` | 1 | **FP** | `sessionMode.isLocked` used as guard — not lock screen functionality |
+| `quality/impossible-state` | 1 | **FP** | Same isLocked guard pattern |
+| `quality/comment-density` | 1 | **FP** | SupportNotifier.js has 40% comments from thorough hand-written JSDoc |
+| `metadata/deprecated-version` | 1 | TP | `version` field ignored by EGO for GNOME 45+ |
+| `quality/file-complexity` | 1 | TP | Manager.js 1,636 non-blank lines |
+| `quality/private-api` | 1 | TP | statusArea access (legitimate for a UI tweaker) |
+| `disclosure/private-api` | 1 | TP | Private API not disclosed in metadata description |
+
+## Fixes Implemented
+
+### False Positives Fixed
+
+| Rule/Check | Root Cause | Fix |
+|------------|-----------|-----|
+| `license` (FAIL) | No parent-dir fallback for src/ layout | `ego-lint.sh`: check parent dir when `basename == "src"` |
+| `metadata/uuid-matches-dir` | Dir is `src` for src/ layout | `check-metadata.py`: skip when `dir_name == "src"` |
+| `session-modes-consistency` | `isLocked` is always a guard | `check-metadata.py`: narrow regex to `sessionMode.currentMode` only |
+| `quality/impossible-state` | `isLocked` is always a guard | `check-quality.py`: remove `isLocked` from check, keep `currentMode` comparisons |
+| `quality/comment-density` | 40% threshold too aggressive | `check-quality.py`: raise threshold from 40% to 50% |
+| `lifecycle/prototype-override` (dupes) | Regex matches override + restore | `check-lifecycle.py`: deduplicate by `(file, prototype)` key |
+
+### Test Fixtures Added
+
+| Fixture | What It Tests |
+|---------|--------------|
+| `src-license-fallback@test` | License found in parent dir for src/ layout; UUID-dir skip |
+| `islocked-guard@test` | `sessionMode.isLocked` guard not flagged as impossible-state or session-modes inconsistency |
+| `proto-override-dedup@test` | Each prototype warned only once despite override + restore pattern |
+
+### Existing Fixture Updated
+
+| Fixture | Change | Reason |
+|---------|--------|--------|
+| `ai-slop@test` | `sessionMode.isLocked` → `sessionMode.currentMode !== 'unlock-dialog'` | `isLocked` no longer triggers impossible-state (by design) |
+
+## Results (after fixes)
+
+| Status | Before | After | Change |
+|--------|--------|-------|--------|
+| FAIL   | 3      | 2     | -1 (license FP fixed) |
+| WARN   | 414    | 408   | -6 (5 FPs + 2 dupes fixed) |
+
+Remaining WARNs breakdown:
+- R-SLOP-01/02: 400 (addressed by pending PR #23)
+- lifecycle/prototype-override: 3 (true positives — advisory)
+- Others: 5 (all true positives)
+
+**With PR #23 merged** (provenance threshold >=3): WARNs would drop to ~8, all true positives.
+
+## Regression Verification
+
+```bash
+bash tests/run-tests.sh
+# Results: 460 passed, 0 failed
+```
+
+| Target | Before | After |
+|--------|--------|-------|
+| Just Perfection | 3 FAIL, 414 WARN | 2 FAIL, 408 WARN |
+| Test suite | 452 passed | 460 passed (+8 new assertions) |
+
+## Calibration Lessons Learned
+
+1. **`sessionMode.isLocked` is always a guard**: Reading lock state to decide behavior is defensive coding, not lock screen functionality. Only `currentMode === 'unlock-dialog'` comparisons suggest active lock screen mode usage.
+
+2. **`src/` layout needs holistic support**: License check and UUID-dir match didn't handle src/ layout. Multiple checks already had src/ fallbacks (CSS, prefs, metadata), but these two were missed. Consider auditing all checks for src/ layout awareness.
+
+3. **Prototype override + restore creates duplicates**: When an extension stores originals and restores them (Just Perfection's `#originals` map pattern), the same regex catches both the override and the restore. Deduplication by `(file, prototype_name)` is essential.
+
+4. **Comment-density threshold interacts with provenance**: Well-documented hand-written code (provenance=3) naturally has higher comment density (40%+) from thorough JSDoc. The 40% threshold was too aggressive; 50% avoids FPs while still catching AI slop (typically 55-70%).
+
+5. **Just Perfection validates PR #23**: Provenance score=3 with 400 JSDoc warnings confirms the threshold change from >=4 to >=3 is needed.
+
+## Issues Created
+
+- [#26](https://github.com/ZviBaratz/gnome-extension-reviewer/issues/26): false-positive: license check fails on src/ layout extensions
+- [#27](https://github.com/ZviBaratz/gnome-extension-reviewer/issues/27): false-positive: isLocked guard flagged as impossible-state and session-modes inconsistency
+- [#28](https://github.com/ZviBaratz/gnome-extension-reviewer/issues/28): bug: prototype-override check emits duplicate warnings

skills/ego-lint/scripts/check-lifecycle.py

@@ -374,15 +374,23 @@ def check_injection_manager(ext_dir):
 
     # WS1-D: Detect direct prototype overrides
     prototype_overrides = []
+    seen_overrides = set()
     for filepath in js_files:
         content = strip_comments(read_file(filepath))
         rel = os.path.relpath(filepath, ext_dir)
         # SomeClass.prototype.methodName = ...
         for m in re.finditer(r'(\w+\.prototype\.\w+)\s*=', content):
-            prototype_overrides.append((rel, m.group(1)))
+            key = (rel, m.group(1))
+            if key not in seen_overrides:
+                seen_overrides.add(key)
+                prototype_overrides.append(key)
         # Object.assign(SomeClass.prototype, ...)
         for m in re.finditer(r'Object\.assign\s*\(\s*(\w+\.prototype)', content):
-            prototype_overrides.append((rel, f"Object.assign({m.group(1)}, ...)"))
+            label = f"Object.assign({m.group(1)}, ...)"
+            key = (rel, label)
+            if key not in seen_overrides:
+                seen_overrides.add(key)
+                prototype_overrides.append(key)
 
     if prototype_overrides:
         # Check if disable() restores prototypes

skills/ego-lint/scripts/check-metadata.py

@@ -173,8 +173,11 @@ def main():
             result("FAIL", "metadata/uuid-format", f"UUID contains invalid characters: {uuid}")
 
         # UUID matches directory name (advisory — cloned repos often differ)
+        # Skip for src/ layout — dirname is "src", not the UUID
         if uuid == dir_name:
             result("PASS", "metadata/uuid-matches-dir", f"UUID matches directory name")
+        elif dir_name == "src":
+            result("PASS", "metadata/uuid-matches-dir", f"UUID check skipped for src/ layout")
         else:
             result("WARN", "metadata/uuid-matches-dir", f"UUID '{uuid}' does not match directory '{dir_name}' — must match when installed")
 
@@ -354,7 +357,9 @@ def check_session_modes_consistency(meta, ext_dir):
                "session-modes includes 'unlock-dialog'")
         return
 
-    session_mode_re = re.compile(r"sessionMode\.(currentMode|isLocked)")
+    # Only flag currentMode checks — isLocked is always a guard pattern
+    # (reading lock state to decide behavior, not evidence of lock screen usage)
+    session_mode_re = re.compile(r"sessionMode\.currentMode")
     found = []
     for root, _dirs, files in os.walk(ext_dir):
         for fname in files:

skills/ego-lint/scripts/check-quality.py

@@ -130,12 +130,10 @@ def check_impossible_state(ext_dir, js_files):
         rel = os.path.relpath(filepath, ext_dir)
         with open(filepath, encoding='utf-8', errors='replace') as f:
             for lineno, line in enumerate(f, 1):
-                if re.search(r'sessionMode\.isLocked', line):
-                    result("WARN", "quality/impossible-state",
-                           f"{rel}:{lineno}: checks isLocked but extension "
-                           f"does not run in lock screen")
-                    found = True
-                elif re.search(r"currentMode\s*===?\s*['\"]unlock-dialog['\"]", line):
+                # sessionMode.isLocked is always a guard (reading lock state
+                # to decide behavior) — not evidence of lock screen mode usage.
+                # Only flag currentMode === 'unlock-dialog' checks.
+                if re.search(r"currentMode\s*[!=]==?\s*['\"]unlock-dialog['\"]", line):
                     result("WARN", "quality/impossible-state",
                            f"{rel}:{lineno}: checks for unlock-dialog but "
                            f"extension does not declare this session-mode")
@@ -699,7 +697,7 @@ def check_comment_density(ext_dir, js_files):
                 code_lines += 1
 
         total = comment_lines + code_lines
-        if total > 0 and comment_lines / total > 0.4:
+        if total > 0 and comment_lines / total > 0.5:
             result("WARN", "quality/comment-density",
                    f"{rel}: {comment_lines}/{total} lines are comments "
                    f"({comment_lines * 100 // total}%) — may indicate AI-generated verbose comments")

skills/ego-lint/scripts/ego-lint.sh

@@ -205,6 +205,17 @@ for candidate in LICENSE COPYING LICENSE.rst LICENSE.md LICENSE.txt COPYING.rst
     fi
 done
 
+# src/ layout fallback: check parent directory for license
+if [[ -z "$license_file" && "$(basename "$EXT_DIR")" == "src" ]]; then
+    parent_dir="$(dirname "$EXT_DIR")"
+    for candidate in LICENSE COPYING LICENSE.rst LICENSE.md LICENSE.txt COPYING.rst COPYING.md COPYING.txt; do
+        if [[ -f "$parent_dir/$candidate" ]]; then
+            license_file="$parent_dir/$candidate"
+            break
+        fi
+    done
+fi
+
 if [[ -n "$license_file" ]]; then
     head_content=$(head -5 "$license_file" 2>/dev/null || true)
     if echo "$head_content" | grep -qiE '(GPL|LGPL|MIT|BSD|Apache|MPL|ISC|Artistic)'; then

tests/assertions/just-perfection-calibration.sh

@@ -0,0 +1,24 @@
+# Field test #10: Just Perfection calibration fixes
+
+# --- src-license-fallback (LICENSE in parent dir of src/) ---
+echo "=== src-license-fallback ==="
+run_lint "src-license-fallback@test/src"
+assert_exit_code "exits with 0 (license found in parent dir)" 0
+assert_output_contains "license found via parent fallback" "\[PASS\].*license"
+assert_output_contains "UUID-dir skipped for src/ layout" "\[PASS\].*metadata/uuid-matches-dir.*src/ layout"
+echo ""
+
+# --- islocked-guard (sessionMode.isLocked as guard, not lock screen) ---
+echo "=== islocked-guard ==="
+run_lint "islocked-guard@test"
+assert_exit_code "exits with 0" 0
+assert_output_not_contains "no impossible-state for isLocked guard" "\[WARN\].*quality/impossible-state"
+assert_output_not_contains "no session-modes-consistency for isLocked" "\[WARN\].*session-modes-consistency"
+echo ""
+
+# --- proto-override-dedup (each prototype warned only once) ---
+echo "=== proto-override-dedup ==="
+run_lint "proto-override-dedup@test"
+assert_output_contains "prototype override detected" "\[WARN\].*lifecycle/prototype-override.*BackgroundMenu.prototype.open"
+assert_output_contains "search prototype override detected" "\[WARN\].*lifecycle/prototype-override.*SearchController.prototype.startSearch"
+echo ""

tests/fixtures/ai-slop@test/extension.js

@@ -15,7 +15,7 @@ export default class AiSlopExtension extends Extension {
         this._pendingDestroy = false;
         this._indicator = null;
 
-        if (!Main.sessionMode.isLocked && this._indicator === null)
+        if (Main.sessionMode.currentMode !== 'unlock-dialog' && this._indicator === null)
             this._initAsync();
     }
 

tests/fixtures/islocked-guard@test/LICENSE

@@ -0,0 +1 @@
+SPDX-License-Identifier: GPL-2.0-or-later

tests/fixtures/islocked-guard@test/extension.js

@@ -0,0 +1,21 @@
+import {Extension} from 'resource:///org/gnome/shell/extensions/extension.js';
+import * as Main from 'resource:///org/gnome/shell/ui/main.js';
+
+export default class TestExtension extends Extension {
+    enable() {
+        if (!this._isLocked()) {
+            this._applySettings();
+        }
+    }
+
+    disable() {
+        this._revertSettings();
+    }
+
+    _isLocked() {
+        return Main.sessionMode.isLocked;
+    }
+
+    _applySettings() {}
+    _revertSettings() {}
+}

tests/fixtures/islocked-guard@test/metadata.json

@@ -0,0 +1,7 @@
+{
+    "uuid": "islocked-guard@test",
+    "name": "Test isLocked Guard",
+    "description": "Tests that sessionMode.isLocked guard is not flagged",
+    "shell-version": ["48"],
+    "url": "https://example.com"
+}