diff --git a/CHANGELOG.md b/CHANGELOG.md index 09040cb..0995dd1 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,6 +6,7 @@ Full release notes with details on each version: [GitHub Releases](https://githu - Feat: opt-in strict PreToolUse hook that actually makes agents use the graph. The installed Claude Code hook has always *nudged* the agent to run `graphify query` before reading raw files, but a nudge is advisory `additionalContext` the model routinely walks past mid-task. `graphify install --project --strict` (or `graphify claude install --strict`) now installs a hook that *blocks* the first raw source read of a session (`permissionDecision: "deny"`) with a redirect to `graphify query`, then downgrades to the soft nudge — so it fires at most once per session and can never strand the agent (the next read proceeds even if no query ran, or if `graphify query` itself failed). Running any `graphify query`/`explain`/`path` refreshes a short-lived "recently oriented" stamp that suppresses the block. Strict mode is Claude Code only (Bash-grep and Glob stay nudge-only; Gemini/Codex/OpenCode can't hard-block and are unchanged); `GRAPHIFY_HOOK_STRICT=1`/`0` toggles it at runtime without a reinstall. Default installs are unchanged (soft nudge). - Fix: the PreToolUse hook stops crying wolf (#1840), which applies to the default soft nudge too. It no longer fires for reads of files **outside** the indexed project (a common false trigger, e.g. a `~/.claude/.../SKILL.md` read), and when the graph is **stale for the target file** (the file changed after the last build, or `graphify watch` flagged the tree) it softens to a non-mandatory nudge that suggests `graphify update` instead of demanding the query. Gating is ~3 `stat` calls — no corpus walk — so it stays fast on large monorepos, and fails open on any error. +- Fix: a same-basename cross-extension re-export no longer manufactures a phantom self-cycle (#1814, thanks @Greg-Moskalenko). A typed `.ts` wrapper that re-exports a hand-written `.mjs` runtime (`export { N } from "./foo.mjs"`) had `foo.ts` and `foo.mjs` collapse onto one base file id (the id stem drops the extension), and while `_disambiguate_colliding_node_ids` correctly salts the two file *nodes* apart (`foo_ts_foo` / `foo_mjs_foo`), the re-export *edge* keyed its target salt by the importer's own source file — mis-pointing the `./foo.mjs` target back at `foo.ts`, a `source == target` self-loop reported as a 1-file import cycle in `GRAPH_REPORT.md`. During disambiguation an import/re-export edge now carries the resolved target file as a *transient* salt key, so the salt lands on the real sibling node (generalizing the C/ObjC `.h`-sibling carve-out from #1475 to every language and to `re_exports`) and the phantom cycle disappears. That hint has no downstream reader and holds an absolute path, so it is popped once consumed and never persisted — and the graph serializer drops it as a backstop — keeping graph.json deterministic and byte-identical across checkout locations. Node ids are unchanged (the residual was purely at the edge layer). One caveat: a graph written by a *pre-fix* build still records the stale self-loop, and because `graphify update` only re-extracts changed files, an unchanged wrapper keeps that edge until it is next edited or a `--force` full rebuild runs — though any stale absolute hint a pre-fix graph happened to persist is dropped on the next build regardless. (The extension-aware-id alternative was rejected: it would rewrite every file and symbol id and force a full-rebuild migration in lockstep with the skill/validation id spec, #1033.) ## 0.9.18 (2026-07-17) diff --git a/graphify/build.py b/graphify/build.py index 9be58fb..6992dbc 100644 --- a/graphify/build.py +++ b/graphify/build.py @@ -706,7 +706,12 @@ def build_from_json(extraction: dict, *, directed: bool = False, root: str | Pat tgt = norm_to_id.get(_normalize_id(tgt), tgt) if src not in node_set or tgt not in node_set: continue # skip edges to external/stdlib nodes - expected, not an error - attrs = {k: v for k, v in edge.items() if k not in ("source", "target")} + # `target_file` is a transient import-disambiguation salt hint (#1814) + # with no downstream reader; it holds an absolute path, so it must never + # be persisted. Disambiguation already pops it off fresh extractions — + # dropping it here as well keeps a pre-fix graph's stale absolute hint + # from surviving an incremental build_merge, which re-serializes base + # edges through here without re-running disambiguation. # Sanitize numeric edge fields (#1960): an explicit ``"weight": null`` in # the extraction JSON survives ``.get("weight", 1.0)`` (the key is present, # so the default never applies) and reaches Louvain/Leiden as None, @@ -716,6 +721,7 @@ def build_from_json(extraction: dict, *, directed: bool = False, root: str | Pat # strings, NaN/inf, negatives — while numeric strings coerce cleanly. # Repair (not drop) the key so graph.json round-trips a clean value and a # cluster-only/--update reload never re-ingests the null. + attrs = {k: v for k, v in edge.items() if k not in ("source", "target", "target_file")} for _num_key in ("weight", "confidence_score"): if _num_key in attrs: try: diff --git a/graphify/extract.py b/graphify/extract.py index 76ed476..bc10f7d 100644 --- a/graphify/extract.py +++ b/graphify/extract.py @@ -309,7 +309,7 @@ def _import_js(node, source: bytes, file_nid: str, stem: str, edges: list, str_p resolved = _resolve_js_import_target(raw, str_path) if resolved is not None: tgt_nid, resolved_path = resolved - edges.append({ + edge = { "source": file_nid, "target": tgt_nid, "relation": "imports_from", @@ -318,7 +318,15 @@ def _import_js(node, source: bytes, file_nid: str, stem: str, edges: list, str_p "source_file": str_path, "source_location": f"L{node.start_point[0] + 1}", "weight": 1.0, - }) + } + # Stamp the resolved target file so a same-basename cross-extension + # sibling (foo.ts importing/re-exporting ./foo.mjs) keys its target salt + # by the TARGET's file rather than the importer's. Both files collapse to + # the base id `foo`; without this the salted lookup mis-points the target + # back onto the importer's own variant, a phantom self-loop (#1814). + if resolved_path is not None: + edge["target_file"] = str(resolved_path) + edges.append(edge) # Emit symbol-level edges for named imports/re-exports from local/aliased files. # e.g. `import { Foo, type Bar } from './bar'` → file → Foo, file → Bar (EXTRACTED) diff --git a/graphify/extractors/engine.py b/graphify/extractors/engine.py index edc624a..e0601cc 100644 --- a/graphify/extractors/engine.py +++ b/graphify/extractors/engine.py @@ -1258,11 +1258,11 @@ def _dynamic_import_js(node, source: bytes, caller_nid: str, str_path: str, edge resolved = _resolve_js_import_target(raw, str_path) if resolved is None: break - tgt_nid, _ = resolved + tgt_nid, resolved_path = resolved pair = (caller_nid, tgt_nid) if pair not in seen_dyn_pairs: seen_dyn_pairs.add(pair) - edges.append({ + edge = { "source": caller_nid, "target": tgt_nid, # A deferred `import(...)` is a real dependency, so keep it as an @@ -1276,7 +1276,12 @@ def _dynamic_import_js(node, source: bytes, caller_nid: str, str_path: str, edge "source_file": str_path, "source_location": f"L{node.start_point[0] + 1}", "weight": 1.0, - }) + } + # Key the target salt by the resolved target file so a same-basename + # cross-extension sibling isn't mis-salted onto the importer (#1814). + if resolved_path is not None: + edge["target_file"] = str(resolved_path) + edges.append(edge) break return True @@ -1570,7 +1575,7 @@ def _require_imports_js(node, source: bytes, file_nid: str, stem: str, edges: li continue tgt_nid, resolved_path = resolved line = node.start_point[0] + 1 - edges.append({ + edge = { "source": file_nid, "target": tgt_nid, "relation": "imports_from", @@ -1579,7 +1584,12 @@ def _require_imports_js(node, source: bytes, file_nid: str, stem: str, edges: li "source_file": str_path, "source_location": f"L{line}", "weight": 1.0, - }) + } + # Key the target salt by the resolved target file so a same-basename + # cross-extension sibling isn't mis-salted onto the importer (#1814). + if resolved_path is not None: + edge["target_file"] = str(resolved_path) + edges.append(edge) found = True # Symbol-level edges for destructured / accessor binders. diff --git a/graphify/extractors/resolution.py b/graphify/extractors/resolution.py index a88abf2..31a2e79 100644 --- a/graphify/extractors/resolution.py +++ b/graphify/extractors/resolution.py @@ -637,6 +637,12 @@ def _disambiguate_colliding_node_ids( node["id"] = new_id if not remap: + # No colliding ids to salt apart, but the transient `target_file` hint an + # importer stamps on every resolved import (#1814) still has to be dropped + # here — this early exit skips the edge loop below, so without it a + # non-colliding import would carry its absolute path into graph.json. + for edge in edges: + edge.pop("target_file", None) return unambiguous_remaps: dict[str, str] = {} @@ -671,7 +677,20 @@ def _disambiguate_colliding_node_ids( for edge in edges: edge_source_key = _source_key(str(edge.get("source_file", "")), root) source_key = (edge.get("source", ""), edge_source_key) - target_key = (edge.get("target", ""), edge_source_key) + # An import/re-export edge's target is a FILE node that can collapse with a + # same-basename cross-extension sibling (foo.ts vs foo.mjs, #1814). Keying + # its target salt by the IMPORTER's own source_file mis-points it back at the + # importer's variant (a self-loop). When the emitter stamped the resolved + # target file, key the target salt by THAT file so the salt lands on the + # correct sibling. Generalizes the #1475 C/ObjC header carve-out (below) to + # every language and to re_exports. `pop` it as we consume it: this is the + # hint's only reader, and its absolute path must not persist into graph.json. + target_file = edge.pop("target_file", None) + if target_file and edge.get("relation") in ("imports", "imports_from", "re_exports"): + target_edge_key = _source_key(str(target_file), root) + else: + target_edge_key = edge_source_key + target_key = (edge.get("target", ""), target_edge_key) if source_key in remap: edge["source"] = remap[source_key] elif edge.get("source") in unambiguous_remaps: @@ -777,12 +796,12 @@ def _apply_symbol_resolution_facts( for edge in edges } - def add_edge(source: str, target: str, relation: str, context: str, line: int, source_path: Path) -> None: + def add_edge(source: str, target: str, relation: str, context: str, line: int, source_path: Path, target_file: str | None = None) -> None: key = (source, target, relation, context or "") if key in existing_edges: return existing_edges.add(key) - edges.append({ + edge = { "source": source, "target": target, "relation": relation, @@ -791,7 +810,13 @@ def _apply_symbol_resolution_facts( "source_file": str(source_path), "source_location": f"L{line}", "weight": 1.0, - }) + } + # A re-export edge's target is a FILE node that can collapse with a + # same-basename cross-extension sibling; stamp the resolved target file so + # the id-disambiguation salt is keyed by the TARGET, not the importer (#1814). + if target_file is not None: + edge["target_file"] = target_file + edges.append(edge) for declaration in facts.declarations: ensure_symbol_node(declaration.file_path, declaration.name, declaration.line) @@ -837,6 +862,7 @@ def _apply_symbol_resolution_facts( "export", star_fact.line, star_fact.file_path, + target_file=str(path_by_resolved.get(target_path, target_path)), ) for namespace_fact in facts.namespace_exports: @@ -867,6 +893,7 @@ def _apply_symbol_resolution_facts( "export", namespace_fact.line, namespace_fact.file_path, + target_file=str(path_by_resolved.get(target_path, target_path)), ) for export_fact in facts.exports: @@ -891,6 +918,7 @@ def _apply_symbol_resolution_facts( "export", export_fact.line, export_fact.file_path, + target_file=str(path_by_resolved.get(origin[0], origin[0])), ) def resolve_exported_origin(target_path: Path, imported_name: str, seen: set[tuple[Path, str]] | None = None) -> tuple[Path, str]: diff --git a/tests/test_cross_extension_reexport_self_cycle.py b/tests/test_cross_extension_reexport_self_cycle.py new file mode 100644 index 0000000..7b66699 --- /dev/null +++ b/tests/test_cross_extension_reexport_self_cycle.py @@ -0,0 +1,311 @@ +"""Same-basename cross-extension re-exports must not collapse to a self-cycle (#1814). + +A hand-written ``.mjs`` plain-ESM runtime plus a thin typed ``.ts`` wrapper that +re-exports it (``export { N } from "./foo.mjs"``) is a common convention. Because +``_file_stem`` drops the extension, ``foo.ts`` and ``foo.mjs`` both collapse to the +base file id ``foo`` at extract time. ``_disambiguate_colliding_node_ids`` salts the +two NODES apart correctly (``foo_ts_foo`` / ``foo_mjs_foo``), but the file-level +re-export edge (and the ``re_exports`` export edge) keyed its target salt by the +IMPORTER's own source_file, mis-pointing the ``./foo.mjs`` target back onto the +importer's own variant — a phantom ``foo.ts -> foo.ts`` self-loop reported as a +1-file import cycle. + +These lock: the salted node ids stay unchanged (the fix does NOT make ids +extension-aware — Option A rejected, #1033), the re-export edge lands on the +sibling node, and no phantom 1-file cycle survives. +""" +from __future__ import annotations + +import json +from pathlib import Path + +from graphify.build import build +from graphify.export import to_json +from graphify.extract import extract + + +def _write(path: Path, text: str) -> Path: + path.parent.mkdir(parents=True, exist_ok=True) + path.write_text(text, encoding="utf-8") + return path + + +def _node_id_by_label(result: dict, label: str) -> str: + ids = [n["id"] for n in result["nodes"] if n.get("label") == label] + assert len(ids) == 1, f"expected exactly one node labelled {label!r}; got {ids}" + return ids[0] + + +def _reexport_like_edges(result: dict) -> list[dict]: + return [ + e for e in result["edges"] + if e.get("relation") in ("imports_from", "re_exports") + ] + + +def test_cross_ext_reexport_emits_no_self_loop(tmp_path: Path): + mjs = _write(tmp_path / "foo.mjs", "export const N = 1;\n") + ts = _write(tmp_path / "foo.ts", 'export { N } from "./foo.mjs";\n') + + result = extract([mjs, ts], cache_root=tmp_path) + + self_loops = [ + e for e in _reexport_like_edges(result) + if e.get("source") == e.get("target") + ] + assert not self_loops, ( + f"cross-extension re-export produced a phantom self-loop; got {self_loops}" + ) + + +def test_cross_ext_reexport_target_is_the_sibling_node(tmp_path: Path): + mjs = _write(tmp_path / "foo.mjs", "export const N = 1;\n") + ts = _write(tmp_path / "foo.ts", 'export { N } from "./foo.mjs";\n') + + result = extract([mjs, ts], cache_root=tmp_path) + + foo_ts = _node_id_by_label(result, "foo.ts") + foo_mjs = _node_id_by_label(result, "foo.mjs") + # The scheme stays as-is: extension dropped from the base stem, siblings salted + # apart by source path. The fix must NOT make node ids extension-aware (#1814 + # Option A rejected). + assert foo_ts == "foo_ts_foo" + assert foo_mjs == "foo_mjs_foo" + + file_level = [ + e for e in result["edges"] + if e.get("relation") == "imports_from" and e.get("source") == foo_ts + ] + assert file_level, "no file-level re-export edge from foo.ts was emitted" + assert all(e.get("target") == foo_mjs for e in file_level), ( + f"file-level re-export must target the .mjs sibling node {foo_mjs!r}; " + f"got {[e.get('target') for e in file_level]}" + ) + + # The symbol-provenance re_exports (context='export') edge must also point at + # the sibling, never back at the importer. + export_edges = [ + e for e in result["edges"] + if e.get("relation") == "re_exports" and e.get("context") == "export" + and e.get("source") == foo_ts + ] + assert export_edges, "no re_exports export edge from foo.ts was emitted" + assert all(e.get("target") == foo_mjs for e in export_edges), ( + f"re_exports export edge must target the .mjs sibling node {foo_mjs!r}; " + f"got {[e.get('target') for e in export_edges]}" + ) + + +def test_cross_ext_reexport_no_phantom_import_cycle(tmp_path: Path): + import networkx as nx + + from graphify.analyze import find_import_cycles + + mjs = _write(tmp_path / "foo.mjs", "export const N = 1;\n") + ts = _write(tmp_path / "foo.ts", 'export { N } from "./foo.mjs";\n') + + result = extract([mjs, ts], cache_root=tmp_path) + + graph = nx.DiGraph() + for node in result["nodes"]: + graph.add_node(node["id"], **{k: v for k, v in node.items() if k != "id"}) + for edge in result["edges"]: + graph.add_edge( + edge["source"], + edge["target"], + **{k: v for k, v in edge.items() if k not in ("source", "target")}, + ) + assert find_import_cycles(graph) == [], ( + "cross-extension re-export must not manufacture a file-level import cycle" + ) + + +def test_same_basename_three_colliding_siblings_reexport_selects_named_variant( + tmp_path: Path, +): + """With three same-basename siblings that all collapse to the base id ``foo`` + (``foo.mjs`` / ``foo.cjs`` / ``foo.ts``), keying the re-export target by the + RESOLVED target file — not the importer's file — must land on the specifically + named ``./foo.mjs`` variant, proving the fix is a real per-file selection and + not a binary coin-flip between two colliders. + + (The issue's illustrative trio uses ``foo.d.mts``, but a ``.d.mts`` stem keeps + its ``.d`` segment and so does NOT collide with ``foo`` — it cannot exercise + multi-variant salt selection. ``foo.cjs`` is the realistic dual-format sibling + that genuinely collides.) + """ + mjs = _write(tmp_path / "foo.mjs", "export const N = 1;\n") + cjs = _write(tmp_path / "foo.cjs", "module.exports.M = 2;\n") + ts = _write(tmp_path / "foo.ts", 'export { N } from "./foo.mjs";\n') + + result = extract([mjs, cjs, ts], cache_root=tmp_path) + + foo_ts = _node_id_by_label(result, "foo.ts") + foo_mjs = _node_id_by_label(result, "foo.mjs") + foo_cjs = _node_id_by_label(result, "foo.cjs") + assert foo_mjs != foo_cjs != foo_ts + + file_level = [ + e for e in result["edges"] + if e.get("relation") == "imports_from" and e.get("source") == foo_ts + ] + assert file_level, "no file-level re-export edge from foo.ts was emitted" + assert all(e.get("target") == foo_mjs for e in file_level), ( + f"re-export of './foo.mjs' must resolve to the .mjs node {foo_mjs!r}, not " + f"the .cjs sibling {foo_cjs!r}; got {[e.get('target') for e in file_level]}" + ) + self_loops = [ + e for e in _reexport_like_edges(result) + if e.get("source") == e.get("target") + ] + assert not self_loops, f"unexpected self-loop among siblings; got {self_loops}" + + +# --------------------------------------------------------------------------- # +# The ``target_file`` the fix stamps on import/re-export edges is a transient +# extraction-time disambiguation salt hint (its only reader is the salt lookup +# in ``_disambiguate_colliding_node_ids``). It carries an ABSOLUTE filesystem +# path, so it must never survive its consumer onto a persisted edge: leaking it +# into graph.json breaks determinism across checkout locations and the +# cross-machine merge/global-graph portability the codebase engineered for. The +# following lock that the hint is stripped after disambiguation and never +# reaches graph.json — on the raw-dump extract path AND the build path — and +# that a persisted absolute hint from a pre-fix graph is dropped on the next +# build rather than carried forward. +# --------------------------------------------------------------------------- # + + +def test_disambiguation_strips_transient_target_file_hint(tmp_path: Path): + mjs = _write(tmp_path / "foo.mjs", "export const N = 1;\n") + ts = _write(tmp_path / "foo.ts", 'export { N } from "./foo.mjs";\n') + + result = extract([mjs, ts], cache_root=tmp_path) + + leaked = [e for e in result["edges"] if "target_file" in e] + assert not leaked, ( + f"the target_file salt hint must not survive disambiguation onto an " + f"edge (it carries an absolute path with no downstream reader); got {leaked}" + ) + + +def test_target_file_hint_stripped_even_without_a_collision(tmp_path: Path): + # No same-basename collision here, so `_disambiguate_colliding_node_ids` + # takes its early `if not remap: return` exit before the edge loop. An + # ordinary import still stamps target_file at extraction, so that early + # exit must strip it too — otherwise every non-colliding import leaks an + # absolute path. + util = _write(tmp_path / "util.ts", "export const helper = 1;\n") + main = _write(tmp_path / "main.ts", 'import { helper } from "./util";\n') + + result = extract([util, main], cache_root=tmp_path) + + leaked = [e for e in result["edges"] if "target_file" in e] + assert not leaked, ( + f"a non-colliding import leaked the transient target_file hint; got {leaked}" + ) + + +def test_graph_json_has_no_target_file_and_no_absolute_path(tmp_path: Path): + mjs = _write(tmp_path / "foo.mjs", "export const N = 1;\n") + ts = _write(tmp_path / "foo.ts", 'export { N } from "./foo.mjs";\n') + + result = extract([mjs, ts], cache_root=tmp_path) + graph = build([result], root=tmp_path) + out = tmp_path / "graph.json" + to_json(graph, {}, str(out), force=True) + + raw = out.read_text(encoding="utf-8") + data = json.loads(raw) + leaked = [link for link in data["links"] if "target_file" in link] + assert not leaked, f"absolute target_file persisted into graph.json links: {leaked}" + assert str(tmp_path.resolve()) not in raw, ( + "graph.json leaked an absolute checkout path (source_file is relativized, " + "but the target_file hint was serialized verbatim)" + ) + + +def test_graph_json_is_checkout_location_independent(tmp_path: Path): + """Building the byte-identical repo at two different absolute locations must + yield identical graph.json edges. A leaked absolute target_file differs by + its checkout prefix and would defeat the cross-machine merge/global-graph + portability the codebase is built around.""" + + def _links_built_at(dirname: str) -> list[dict]: + d = tmp_path / dirname + d.mkdir() + mjs = _write(d / "foo.mjs", "export const N = 1;\n") + ts = _write(d / "foo.ts", 'export { N } from "./foo.mjs";\n') + result = extract([mjs, ts], cache_root=d) + graph = build([result], root=d) + out = d / "graph.json" + to_json(graph, {}, str(out), force=True) + links = json.loads(out.read_text(encoding="utf-8"))["links"] + return sorted( + links, + key=lambda link: ( + str(link.get("source")), + str(link.get("target")), + str(link.get("relation")), + ), + ) + + assert _links_built_at("loc_a") == _links_built_at("loc_bbbb_longer"), ( + "graph.json edges differ across checkout locations — an absolute path leaked" + ) + + +def test_build_drops_persisted_target_file_from_a_pre_fix_graph(tmp_path: Path): + # A graph.json written by a pre-fix build carries an absolute target_file on + # its import edges. On the next (incremental) build those base edges are + # re-serialized through build(), which does NOT re-run disambiguation — so + # the serializer itself must drop the persisted absolute path rather than + # carry a foreign checkout prefix forward into the updated graph. + legacy_chunk = { + "nodes": [ + {"id": "foo_ts_foo", "label": "foo.ts", + "source_file": "foo.ts", "file_type": "code"}, + {"id": "foo_mjs_foo", "label": "foo.mjs", + "source_file": "foo.mjs", "file_type": "code"}, + ], + "edges": [ + { + "source": "foo_ts_foo", + "target": "foo_mjs_foo", + "relation": "imports_from", + "context": "re-export", + "confidence": "EXTRACTED", + "source_file": "foo.ts", + "target_file": "/some/other/checkout/foo.mjs", + "weight": 1.0, + } + ], + } + + graph = build([legacy_chunk], root=tmp_path) + + assert graph.number_of_edges() == 1, "the base import edge should survive the merge" + for _src, _tgt, data in graph.edges(data=True): + assert "target_file" not in data, ( + f"build() carried a persisted absolute target_file into the graph: {data}" + ) + + +def test_target_file_hint_never_written_to_the_ast_cache(tmp_path: Path): + """The hint is emitted only on JS/TS-family edges, and those suffixes bypass + the AST cache entirely (``_JS_CACHE_BYPASS_SUFFIXES``). A warm/relocated + cache therefore can never carry a foreign absolute target_file that would + miss the disambiguation salt. Lock that no AST cache entry stores it.""" + mjs = _write(tmp_path / "foo.mjs", "export const N = 1;\n") + ts = _write(tmp_path / "foo.ts", 'export { N } from "./foo.mjs";\n') + + extract([mjs, ts], cache_root=tmp_path) + + ast_dir = tmp_path / "graphify-out" / "cache" / "ast" + entries = list(ast_dir.rglob("*.json")) if ast_dir.exists() else [] + for entry in entries: + payload = json.loads(entry.read_text(encoding="utf-8")) + for edge in payload.get("edges", []): + assert "target_file" not in edge, ( + f"AST cache entry {entry.name} stored a non-portable target_file " + f"hint: {edge}" + )