fix: make ref-check's suggestions resolve, and cut its false positives (#94)

Co-authored-by: Claude Opus 5.5 (1M context) <[email protected]>

README.md

@@ -220,7 +220,8 @@ A target is a file, a `dir/`, `glob:<pattern>`, `file#symbol`, or `doc.md#headin
 never a line number (line numbers rot without any signal). The check is strict on
 purpose: any backticked token in `.expo-agents/code-review/` that names something that
 exists in the repo **must** be a ref, because stale citations are exactly the ones
-nobody annotated. Mark the rare false positive once with
+nobody annotated. A trailing-slash name like `ios/` that exists only inside packages
+reads as "each package's `ios/`" and is left alone. Mark the rare false positive once with
 `<!-- @ref-ignore knex.raw() -->`. Refs are always repo-root-relative, including in
 a scope's own setup dir, and config declarations (`enforceAgents` ids, scope
 directories and globs) are checked too.
@@ -230,7 +231,8 @@ Two run points:
 - `ecr ref-check` exits 1 on any problem. Run it in CI or a pre-commit hook.
 - `ecr review` / `ecr ci` run it too but never fail a PR's checks with it — broken
   refs (and cited code *this PR* changes) surface as a **Review setup** note in the
-  comment.
+  comment. A ref to a top-level directory (`packages/`) is checked but too broad to
+  drive the "this PR changes cited code" note.
 
 Full detail: [LLP 0012](./llp/0012-config-ref-integrity.explainer.md).
 

llp/0012-config-ref-integrity.explainer.md

@@ -82,8 +82,11 @@ config keys, severities, commands and model ids. It comes in two tiers.
 **Shape alone** (`isCodeCitation`): an allowlisted source extension, a trailing slash, or
 a wildcard tail inside a path — and never whitespace, `:`, `<`, `(` or quotes. So
 `session.ts`, `src/entities/oauth/` and `.github/workflows/**` are citations, while
-`ecr ci`, `label:<agent>`, `knex.raw()`, `critical` and a bare `.ts` are prose. Fenced
-code blocks are skipped whole: they are examples, not claims about the tree.
+`ecr ci`, `label:<agent>`, `knex.raw()`, `critical` and a bare `.ts` are prose. The
+allowlist carries native build files (`.mm`, `.gradle`, `.xml`, `.plist`, `.podspec`, …),
+which mobile prompts cite constantly; `.env` stays off it, because `process.env` is the
+commoner token. Fenced code blocks are skipped whole: they are examples, not claims about
+the tree.
 
 **Shape plus the filesystem** (`pathishCandidates`), for the citations shape cannot
 settle. A reviewer of this feature named the case: a Terraform prompt citing
@@ -97,6 +100,29 @@ lists cut back to their fixed prefix. A token carrying `*` resolves as a glob in
 scope-rebased when that is what matches, so the suggested `glob:` ref itself resolves.
 Model ids resolve to nothing and stay prose.
 
+A trailing slash is shape, but not enough on its own. Prompts for a repo of many
+packages say `ios/`, `src/` or `android/src/` meaning *each package's* directory of that
+name, and no single ref can pin that: `ios/` does not exist at the root, and
+`glob:**/ios/**` matches the whole tree, so it would verify nothing. So a trailing-slash
+token that resolves at neither the scope root nor the repo root is a citation only when
+no directory of that name exists anywhere in the repo; if one does, it is
+package-relative prose. Swept against expo/expo this removed 28 of 149 problems, every
+one of them a package-relative name whose suggested fix could never resolve [observed].
+The tradeoff is the extensionless one again: a misspelled `iso/` or a deleted `oldpkg/`
+still fails, but a stale `foo/bar/` whose name survives somewhere else
+in the tree goes quiet.
+
+Every suggested fix must itself resolve, or following the message trades one failure
+for another. A partial path (`start/doctor/apple/XcodePrerequisite.ts`) is taught the
+one repo file it is a suffix of, or `glob:**/<token>` when several are, and a suffix glob
+covers the partial path it was written for: coverage reads each `**/` as zero or more
+directories, as routing does ([LLP 0004][dialect]), while resolution keeps the shared
+dialect unchanged, so `./ref-check` needs no change. A wildcard directory
+(`packages/*/apple/`) is taught `glob:packages/*/apple/**`, since a glob matches files
+only, and a leading `./` is dropped, since repo paths never carry one. A citation that matches nothing at
+all gets no suggestion: the message says it does not resolve and may be stale, which is
+usually the real finding.
+
 The consequence is deliberate: an extensionless path that had already drifted before
 adoption is invisible (nothing distinguishes it from prose), while every one that is true
 today gets pinned and will be caught the moment it moves. This mechanism prevents future
@@ -136,8 +162,19 @@ The review path also runs it, and never fails a PR's checks with it — that inv
 holds ([LLP 0002][pipeline]). Instead the review *advises*: broken refs in the setup
 become a note the reviewer comment carries, and `citedPathsTouchedBy` adds the drift
 signal a static check cannot give — "this PR changes code your reviewer prompts cite, so
-the guidance may need updating" — even when the ref still resolves. In CI the config
-comes from the trusted base ref while refs resolve against the PR head tree, so a PR that
+the guidance may need updating" — even when the ref still resolves. Only refs that fail
+to resolve count toward the first note; a line-number citation is refused by the command
+but usually names a file that exists, so it is not reported as unresolved. The drift
+signal matches by prefix, so a ref to a top-level directory (`packages/`, `docs/`) is
+still checked but does not feed it. With every suggested ref followed, plus `packages/`,
+`docs/` and `apps/`, the note fired on 282 of expo/expo's last 300 commits; without the
+top-level directories, 196 [observed]. A note that fires on nearly every PR is noise.
+The cut stops at one segment because two (`packages/expo-ui/`) usually name one unit
+someone owns, where a PR touching it is the signal working; the remaining volume comes
+mostly from such package directories, and from wide two-segment ones like
+`docs/pages/`, which an author should narrow by hand.
+
+In CI the config comes from the trusted base ref while refs resolve against the PR head tree, so a PR that
 moves cited code is reported against the config that will actually review it.
 
 ## Two Checkers, One Grammar
@@ -172,6 +209,7 @@ the glob dialect must land in `./ref-check` and `src/core/config-refs.ts` togeth
 same annotation will pass one checker and fail the other.
 
 [verify]: 0007-cli-commands-and-ci.explainer.md#verify-config-the-config-guard
+[dialect]: 0004-diff-noise-and-prompts.explainer.md#the-mini-glob-dialect
 [verify5]: 0005-verification-fingerprints-rendering.explainer.md#verifier-confinement-and-fail-open
 [routing]: 0006-config-schema-loading-routing.explainer.md#routing-manifest
 [pipeline]: 0002-review-engine-pipeline.explainer.md#pipeline-stages

src/__tests__/config-refs.test.ts

@@ -554,3 +554,177 @@ test("routing: a glob counts as live under the same matcher scope assignment use
   const report = await checkConfigRefs({ root });
   expect(report.problems.filter((p) => p.problem.includes("matches no file"))).toEqual([]);
 });
+
+// ---------------------------------------------------------------------------
+// precision: native files, suggestions that resolve, package-relative dirs
+// ---------------------------------------------------------------------------
+
+test("native build files are citations by shape; process.env is not", () => {
+  for (const token of [
+    "RCTFontUtils.mm",
+    "ios/EXFoo.m",
+    "android/build.gradle",
+    "build.gradle.kts",
+    "android/src/main/AndroidManifest.xml",
+    "Info.plist",
+    "ExpoModulesCore.podspec",
+    "ios/App.xcodeproj/project.pbxproj",
+    "Debug.xcconfig",
+    "App.entitlements",
+    "gradle.properties",
+    "toolchain.cmake",
+    "proguard-rules.pro",
+    "docs/pages/index.mdx",
+  ]) {
+    expect(isCodeCitation(token)).toBe(true);
+  }
+  expect(isCodeCitation("process.env")).toBe(false);
+});
+
+test("a suffix glob ref covers the partial path it was written for", async () => {
+  const root = await makeRepo(
+    setup({
+      "packages/cli/src/start/doctor/XcodePrerequisite.ts": "export {}\n",
+      ".expo-code-review/agents/security.md":
+        `<!-- ${REF} glob:**/start/doctor/XcodePrerequisite.ts — the doctor check -->\n` +
+        "See `start/doctor/XcodePrerequisite.ts`.\n",
+    }),
+  );
+  expect((await checkConfigRefs({ root })).problems).toEqual([]);
+
+  // Once the file is gone, the glob ref is the one problem: the prose it covers is not
+  // reported a second time (no resolved file stands in for the token here).
+  const gone = await makeRepo(
+    setup({
+      "packages/cli/src/index.ts": "export {}\n",
+      ".expo-code-review/agents/security.md":
+        `<!-- ${REF} glob:**/start/doctor/XcodePrerequisite.ts — the doctor check -->\n` +
+        "See `start/doctor/XcodePrerequisite.ts`.\n",
+    }),
+  );
+  expect((await checkConfigRefs({ root: gone })).problems.map((problem) => problem.kind)).toEqual([
+    "broken-ref",
+  ]);
+});
+
+test("a partial path is taught the one file it names, or a suffix glob when several", async () => {
+  const files = (pin: string, extra: Record<string, string> = {}): Record<string, string> =>
+    setup({
+      "packages/cli/src/export/embed/exportServer.ts": "export {}\n",
+      ...extra,
+      ".expo-code-review/agents/security.md": `${pin}\nSee \`export/embed/exportServer.ts\`.\n`,
+    });
+
+  const unique = await checkConfigRefs({ root: await makeRepo(files("")) });
+  expect(unique.problems).toHaveLength(1);
+  expect(unique.problems[0]!.problem).toContain(
+    `add \`${REF} packages/cli/src/export/embed/exportServer.ts —`,
+  );
+  const uniquePinned = await checkConfigRefs({
+    root: await makeRepo(
+      files(`<!-- ${REF} packages/cli/src/export/embed/exportServer.ts — the server -->`),
+    ),
+  });
+  expect(uniquePinned.problems).toEqual([]);
+
+  const twin = { "packages/other/export/embed/exportServer.ts": "export {}\n" };
+  const several = await checkConfigRefs({ root: await makeRepo(files("", twin)) });
+  expect(several.problems[0]!.problem).toContain(
+    `add \`${REF} glob:**/export/embed/exportServer.ts —`,
+  );
+  const severalPinned = await checkConfigRefs({
+    root: await makeRepo(
+      files(`<!-- ${REF} glob:**/export/embed/exportServer.ts — both servers -->`, twin),
+    ),
+  });
+  expect(severalPinned.problems).toEqual([]);
+});
+
+test("a wildcard directory or ./ glob is taught a glob that matches files", async () => {
+  const files = (pin: string): Record<string, string> =>
+    setup({
+      "packages/a/apple/A.swift": "struct A {}\n",
+      "internal/x.ts": "export {}\n",
+      ".expo-code-review/agents/ios.md": `${pin}\nSee \`packages/*/apple/\` and \`./internal/*\`.\n`,
+    });
+  const report = await checkConfigRefs({ root: await makeRepo(files("")) });
+  expect(report.problems.map((problem) => problem.problem)).toEqual([
+    expect.stringContaining(`add \`${REF} glob:internal/* —`),
+    expect.stringContaining(`add \`${REF} glob:packages/*/apple/** —`),
+  ]);
+  const pinned = await checkConfigRefs({
+    root: await makeRepo(
+      files(
+        `<!-- ${REF} glob:packages/*/apple/** — the apple dirs -->\n` +
+          `<!-- ${REF} glob:internal/* — the internals -->`,
+      ),
+    ),
+  });
+  expect(pinned.problems).toEqual([]);
+});
+
+test("a citation that matches nothing says so instead of suggesting a broken ref", async () => {
+  const root = await makeRepo(
+    setup({
+      "src/a.ts": "export {}\n",
+      ".expo-code-review/agents/api.md":
+        "See `tools/src/publish-packages/helpers.ts`, `gone.ts` and `tools/gone.ts:12`.\n",
+    }),
+  );
+  const problems = (await checkConfigRefs({ root })).problems.map((problem) => problem.problem);
+  expect(problems).toHaveLength(3);
+  for (const problem of problems) {
+    expect(problem).toContain("matches nothing in the repo");
+    expect(problem).not.toContain(`add \`${REF}`);
+    expect(problem).not.toContain(`as \`${REF}`);
+  }
+});
+
+test("a trailing-slash name that exists somewhere is package-relative prose; one that exists nowhere fails", async () => {
+  const root = await makeRepo(
+    setup({
+      "packages/a/ios/A.swift": "struct A {}\n",
+      "packages/b/android/src/B.kt": "class B\n",
+      ".expo-code-review/agents/native.md":
+        "Each package's `ios/` and `android/src/` dirs; never `iso/`.\n",
+    }),
+  );
+  const problems = (await checkConfigRefs({ root })).problems;
+  expect(problems.map((problem) => problem.target)).toEqual(["iso/"]);
+  expect(problems[0]!.problem).toContain("matches nothing in the repo");
+  expect(problems[0]!.problem).not.toContain("glob:");
+});
+
+test("a top-level directory ref must resolve but does not feed the drift note", async () => {
+  const root = await makeRepo(
+    setup({
+      "package.json": "{}\n",
+      "packages/a/index.ts": "export {}\n",
+      ".expo-code-review/agents/security.md":
+        `<!-- ${REF} packages/ — every package -->\n` +
+        `<!-- ${REF} packages/a/ — one package -->\n` +
+        `<!-- ${REF} package.json — the root manifest -->\n` +
+        `<!-- ${REF} gone/ — not there -->\n`,
+    }),
+  );
+  const report = await checkConfigRefs({ root });
+  expect(report.problems.map((problem) => problem.target)).toEqual(["gone/"]);
+  expect(report.citedPaths).toEqual(["package.json", "packages/a"]);
+  expect(citedPathsTouchedBy(report, ["packages/b/index.ts"])).toEqual([]);
+});
+
+test("a line-number citation of a file that exists is not called unresolvable in the review", async () => {
+  const root = await makeRepo(
+    setup({
+      "src/a.ts": "export {}\n",
+      ".expo-code-review/agents/security.md": `<!-- ${REF} src/a.ts:3 — pinned -->\nSee \`src/a.ts:42\`.\n`,
+    }),
+  );
+  const setupDirs = [path.join(root, ".expo-code-review")];
+  expect(await reviewSetupRefNotes({ root, setupDirs, changedFiles: [] })).toEqual([]);
+  // The command still refuses both.
+  expect((await checkConfigRefs({ root })).problems.map((problem) => problem.kind)).toEqual([
+    "line-number-ref",
+    "line-number-ref",
+  ]);
+});

src/core/config-refs.ts

@@ -42,16 +42,22 @@ const LINE_CITATION_RE = /^(.+?):(\d+)(?:-\d+)?$/;
 /**
  * Extensions that make a backticked token a *code citation* rather than prose.
  * Deliberately an allowlist: `openai/gpt-5.5` and `label:<agent>` must not read as
- * paths, while `session.ts` and `.github/workflows/**` must.
+ * paths, while `session.ts` and `.github/workflows/**` must. Native build files are on
+ * it (`RCTFontUtils.mm`, `build.gradle`, `AndroidManifest.xml`, `Info.plist`,
+ * `proguard-rules.pro`) because mobile prompts cite them constantly; `.env` is not,
+ * since `process.env` is the far more common backticked token.
  */
 const CITATION_EXTENSIONS = new Set([
   ".c",
   ".cc",
   ".cjs",
+  ".cmake",
   ".cpp",
   ".cs",
   ".css",
+  ".entitlements",
   ".go",
+  ".gradle",
   ".graphql",
   ".hcl",
   ".h",
@@ -63,12 +69,21 @@ const CITATION_EXTENSIONS = new Set([
   ".jsonc",
   ".jsx",
   ".kt",
+  ".kts",
   ".lock",
+  ".m",
   ".md",
+  ".mdx",
   ".mjs",
+  ".mm",
   ".mts",
+  ".pbxproj",
   ".php",
+  ".plist",
+  ".podspec",
   ".prisma",
+  ".pro",
+  ".properties",
   ".proto",
   ".py",
   ".rb",
@@ -84,6 +99,8 @@ const CITATION_EXTENSIONS = new Set([
   ".tsx",
   ".txt",
   ".vue",
+  ".xcconfig",
+  ".xml",
   ".yaml",
   ".yml",
   ".zsh",
@@ -454,6 +471,14 @@ function fileLabel(root: string, setupDir: string, file: string): string {
   return path.join(CONFIG_DIRNAME, path.relative(setupDir, file));
 }
 
+/** What prose adds to a path that a repo path never has: `.../`, `./`, a leading `/`. */
+function barePath(token: string): string {
+  return token
+    .replace(/^\.\.\.\/?/, "")
+    .replace(/^(?:\.\/)+/, "")
+    .replace(/^\/+/, "");
+}
+
 function splitAnchor(target: string): [string, string | undefined] {
   const hash = target.indexOf("#");
   return hash === -1 ? [target, undefined] : [target.slice(0, hash), target.slice(hash + 1)];
@@ -748,6 +773,65 @@ export async function checkConfigRefs(options: CheckConfigRefsOptions): Promise<
     return null;
   };
 
+  // @ref LLP 0012#unannotated-citations-are-broken-refs — the ref a message suggests must itself resolve
+  /**
+   * The ref an unannotated citation should become, chosen so that writing it down
+   * resolves: the path or glob it names, else the one repo file it is a suffix of, else
+   * a suffix glob when several are. Null when nothing in the repo matches: the citation
+   * itself is stale, and any ref to it would be broken too.
+   */
+  const resolvingRef = (
+    citation: string,
+    scopeRoot: string,
+    named: string | null,
+  ): string | null => {
+    const isWild = citation.includes("*");
+    if (named) {
+      return isWild ? `${GLOB_PREFIX}${named}` : named;
+    }
+    if (index.files.length === 0) {
+      return suggestedRef(citation); // no file list (not a git checkout) — uncheckable
+    }
+    const bare = barePath(citation);
+    if (!bare) {
+      return null;
+    }
+    if (isWild) {
+      // A glob matches files only, so a directory pattern (`packages/*/apple/`) needs a
+      // `**` tail to match anything; a leading `./` never matches (paths have none).
+      const pattern = bare.endsWith("/") ? `${bare}**` : bare;
+      const glob =
+        namedGlob(pattern, scopeRoot) ??
+        (pattern.startsWith("**/") ? null : namedGlob(`**/${pattern}`, scopeRoot));
+      return glob ? `${GLOB_PREFIX}${glob}` : null;
+    }
+    if (bare.endsWith("/")) {
+      return null; // a directory that exists nowhere (one that exists somewhere is prose)
+    }
+    const matches = index.files.filter((file) => file === bare || file.endsWith(`/${bare}`));
+    if (matches.length === 0) {
+      return null;
+    }
+    if (matches.includes(bare)) {
+      return bare;
+    }
+    // A partial path that names exactly one file gets that file. A bare or `...`-
+    // abbreviated filename keeps its short suffix glob, which is why that form exists.
+    if (matches.length === 1 && bare.includes("/") && !citation.startsWith("...")) {
+      return matches[0]!;
+    }
+    return `${GLOB_PREFIX}**/${bare}`;
+  };
+
+  /** Does a directory with this (partial) path exist anywhere — `**\/<dir>/**`? */
+  const dirExistsAnywhere = (token: string): boolean => {
+    const bare = barePath(token);
+    return (
+      bare.length > 1 &&
+      index.files.some((file) => file.startsWith(bare) || file.includes(`/${bare}`))
+    );
+  };
+
   for (const dir of dirs) {
     const scopeRoot = path.dirname(dir);
     for (const file of await setupFiles(dir)) {
@@ -788,10 +872,15 @@ export async function checkConfigRefs(options: CheckConfigRefsOptions): Promise<
           continue;
         }
         refs.push(ref);
-        const cited = splitAnchor(ref.target)[0];
-        if (cited && !cited.startsWith(GLOB_PREFIX) && !URL_RE.test(cited)) {
-          citedPaths.add(cited.replace(/\/$/, ""));
+        const cited = splitAnchor(ref.target)[0].replace(/\/$/, "");
+        if (!cited || cited.startsWith(GLOB_PREFIX) || URL_RE.test(cited)) {
+          continue;
         }
+        // @ref LLP 0012#run-points-command-and-review — a top-level dir is checked, but too broad to signal drift
+        if (!cited.includes("/") && (await pathKind(path.resolve(root, cited))) === "dir") {
+          continue;
+        }
+        citedPaths.add(cited);
       }
 
       for (const { line, token } of findProseCitations(text)) {
@@ -802,32 +891,51 @@ export async function checkConfigRefs(options: CheckConfigRefsOptions): Promise<
         if (!isCodeCitation(token) && !named) {
           continue;
         }
-        // Coverage accepts the resolved path too, else the only ref that silences a
-        // scope-relative citation is an over-broad `glob:**/<basename>`.
-        const forms = [...citationForms(token), ...(named ? coveringForms(named) : [])];
+        const lineCitation = LINE_CITATION_RE.exec(token);
+        // @ref LLP 0012#unannotated-citations-are-broken-refs — a dir that exists somewhere is a package-relative name, not a citation
+        if (!named && !isWild && !lineCitation && token.endsWith("/") && dirExistsAnywhere(token)) {
+          continue;
+        }
+        const cited = lineCitation ? lineCitation[1]! : token;
+        const suggestion = resolvingRef(
+          cited,
+          scopeRoot,
+          lineCitation ? await namedPath(cited, scopeRoot) : named,
+        );
+        // Coverage accepts the resolved path and the suggested ref too, so writing the
+        // suggestion down silences the citation, and a scope-relative citation is not
+        // left needing an over-broad `glob:**/<basename>`.
+        const forms = [
+          ...citationForms(token),
+          ...(named ? coveringForms(named) : []),
+          ...(suggestion ? coveringForms(suggestion) : []),
+        ];
         if (
           ignored.has(token) ||
           forms.some((form) => covered.has(form)) ||
           coveringGlobs.some((pattern) =>
-            forms.some((form) => matchesIgnore(form.replace(/^\.\.\.\//, ""), pattern)),
+            // @ref LLP 0004#the-mini-glob-dialect — coverage reads `**/` as zero or more dirs (the routing variant), so `glob:**/a/B.kt` covers `a/B.kt`
+            forms.some((form) => matchesPathGlobs([pattern], form.replace(/^\.\.\.\//, ""))),
           )
         ) {
           continue;
         }
-        const lineCitation = LINE_CITATION_RE.exec(token);
-        // A wildcard token means the family; anything else gets the precise resolved path.
-        const suggestion = isWild
-          ? named
-            ? `${GLOB_PREFIX}${named}`
-            : suggestedRef(token)
-          : (named ?? suggestedRef(token));
+        const ignoreHint = `(or \`${IGNORE_MARK} ${token}\` if it is not a path)`;
+        let problem: string;
+        if (lineCitation) {
+          problem = suggestion
+            ? `\`${token}\` pins a line number; cite the file or a \`#symbol\` instead, as \`${REF_MARK} ${suggestion}\``
+            : `\`${token}\` pins a line number, and \`${cited}\` matches nothing in the repo; update the prompt to cite where that code lives now, by file or \`#symbol\``;
+        } else {
+          problem = suggestion
+            ? `\`${token}\` cites code without a ref; add \`${REF_MARK} ${suggestion} — why it matters\` ${ignoreHint}`
+            : `\`${token}\` matches nothing in the repo, so no ref to it can resolve; the citation may be stale — update the prompt if that code moved or was deleted ${ignoreHint}`;
+        }
         problems.push({
           file: relative,
           line,
           kind: lineCitation ? "line-number-ref" : "unannotated-citation",
-          problem: lineCitation
-            ? `\`${token}\` pins a line number; cite the file or a \`#symbol\` instead, as \`${REF_MARK} ${suggestedRef(lineCitation[1]!)}\``
-            : `\`${token}\` cites code without a ref; add \`${REF_MARK} ${suggestion} — why it matters\` (or \`${IGNORE_MARK} ${token}\` if it is not a path)`,
+          problem,
           target: token,
         });
       }
@@ -892,9 +1000,11 @@ export async function reviewSetupRefNotes(options: {
   // The read root is scrubbed of ambient runtime config (AGENTS.md, CLAUDE.md, .env…),
   // so a ref to one is missing by design here, not stale. The template itself cites
   // AGENTS.md, which would make this note cry wolf on every PR.
+  // Only refs that fail to resolve: a line-number citation (annotated or prose) points at
+  // a file that may well exist, and an unannotated citation is a gate failure, not advice.
   const broken = report.problems.filter(
     (problem) =>
-      problem.kind !== "unannotated-citation" &&
+      (problem.kind === "broken-ref" || problem.kind === "structural") &&
       !(problem.target && isAmbientRuntimeConfig(citedBasename(problem.target))),
   );
   if (broken.length > 0) {