mirror of
https://github.com/safishamsi/graphify.git
synced 2026-09-24 22:45:43 +00:00
fix: obsidian crash, NFC/NFD dedup, JSON data nodes, OpenAI temperature, JSON config detection
- export.py: guard to_obsidian/to_canvas against dangling community member IDs (KeyError crash when a node in communities dict is absent from graph, #1236) - detect.py: NFC-normalize path before hashing Office sidecar filename to fix macOS NFC/NFD mismatch causing --update to re-extract all Office files (#1226) - extract.py: add _is_config_json() to skip data JSON files (only extract package.json, tsconfig.json, eslint, deno, JSON Schema etc.) eliminating 561 orphan key-nodes on large repos (#1224) - llm.py: add GRAPHIFY_LLM_TEMPERATURE env var + _resolve_temperature() helper; auto-omit temperature for o1/o3/o4/gpt-5 reasoning models that reject temp=0; mirrors GRAPHIFY_MAX_OUTPUT_TOKENS precedence pattern (#1191) - tests: 20 new regression tests across obsidian, detect, extract, llm_backends Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
This commit is contained in:
co-authored by
Claude Sonnet 4.6
parent
5504c84324
commit
a37672f25f
+14
-2
@@ -610,10 +610,22 @@ def convert_office_file(path: Path, out_dir: Path) -> Path | None:
|
||||
return None
|
||||
|
||||
out_dir.mkdir(parents=True, exist_ok=True)
|
||||
# Use a stable name derived from the original path to avoid collisions
|
||||
# Use a stable name derived from the original path to avoid collisions.
|
||||
# Normalize the resolved path to NFC before hashing: on macOS (HFS+/APFS)
|
||||
# os.walk/rglob return filenames in NFD, while Python string literals and
|
||||
# directly-constructed Path objects are NFC, so the same source file would
|
||||
# otherwise hash to different sidecar names across runs — causing --update
|
||||
# to treat every Office file as new and re-extract it (#1226).
|
||||
import hashlib
|
||||
name_hash = hashlib.sha256(str(path.resolve()).encode()).hexdigest()[:8]
|
||||
import unicodedata
|
||||
normalized_path = unicodedata.normalize("NFC", str(path.resolve()))
|
||||
name_hash = hashlib.sha256(normalized_path.encode()).hexdigest()[:8]
|
||||
out_path = out_dir / f"{path.stem}_{name_hash}.md"
|
||||
# Once the hash is stable the sidecar name is deterministic; skip re-writing
|
||||
# an existing sidecar so an unchanged source never churns its mtime (which
|
||||
# would still flag it as changed in detect_incremental).
|
||||
if out_path.exists():
|
||||
return out_path
|
||||
out_path.write_text(
|
||||
f"<!-- converted from {path.name} -->\n\n{text}",
|
||||
encoding="utf-8",
|
||||
|
||||
+7
-1
@@ -972,12 +972,18 @@ def to_obsidian(
|
||||
return len(neighbor_cids)
|
||||
|
||||
community_notes_written = 0
|
||||
for cid, members in communities.items():
|
||||
for cid, all_members in communities.items():
|
||||
community_name = (
|
||||
community_labels.get(cid, f"Community {cid}")
|
||||
if community_labels and cid is not None
|
||||
else f"Community {cid}"
|
||||
)
|
||||
# A community's member list can contain ids with no backing node in G
|
||||
# (e.g. pruned nodes, stale community assignments from a prior run, or
|
||||
# synthesized/merge-artifact ids). Dereferencing those via G.nodes[n] or
|
||||
# node_filename[n] raises KeyError and aborts the whole vault export, so
|
||||
# skip dangling members rather than crashing (issue #1236).
|
||||
members = [m for m in all_members if m in G and m in node_filename]
|
||||
n_members = len(members)
|
||||
coh_value = cohesion.get(cid) if cohesion else None
|
||||
|
||||
|
||||
+65
-1
@@ -10202,8 +10202,64 @@ def extract_razor(path: Path) -> dict:
|
||||
return {"nodes": nodes, "edges": edges}
|
||||
|
||||
|
||||
# Config/manifest JSON filenames the structural extractor understands. Anything
|
||||
# else (eval fixtures, datasets, GeoJSON, API dumps) is *data* and must NOT be
|
||||
# AST-walked into per-key nodes — that floods the graph with orphan key-nodes
|
||||
# and near-duplicate communities (#1224). Data JSON is left to the LLM semantic
|
||||
# pass instead. Matched case-insensitively against the bare filename.
|
||||
_CONFIG_JSON_NAMES = frozenset({
|
||||
"package.json", "tsconfig.json", "jsconfig.json", "composer.json",
|
||||
"deno.json", "deno.jsonc", "bower.json", "manifest.json",
|
||||
"app.json", "now.json", "vercel.json", "angular.json", "nest-cli.json",
|
||||
"biome.json", "biome.jsonc", "renovate.json", ".babelrc", ".babelrc.json",
|
||||
".eslintrc.json", ".prettierrc.json", ".prettierrc", "babel.config.json",
|
||||
})
|
||||
|
||||
# Top-level keys that prove a JSON object is a config/manifest the extractor can
|
||||
# draw *cross-file* edges from (deps, extends chains, schema refs).
|
||||
_CONFIG_JSON_KEYS = frozenset({
|
||||
"dependencies", "devDependencies", "peerDependencies",
|
||||
"optionalDependencies", "bundleDependencies", "bundledDependencies",
|
||||
"extends", "$ref", "$schema", "compilerOptions",
|
||||
})
|
||||
|
||||
|
||||
def _is_config_json(path: Path, obj_node, source: bytes) -> bool:
|
||||
"""True if a .json file is a recognized config/manifest worth AST-extracting.
|
||||
|
||||
Matches by filename first (cheap), then falls back to a top-level key probe
|
||||
so arbitrarily-named config files (e.g. ``api.tsconfig.json``,
|
||||
``foo.eslintrc.json``) are still picked up. Returns False for data JSON so it
|
||||
is skipped by the structural pass (#1224)."""
|
||||
name = path.name.casefold()
|
||||
if name in _CONFIG_JSON_NAMES:
|
||||
return True
|
||||
# Common compound config names: *.eslintrc.json, *.prettierrc.json, etc.
|
||||
if name.endswith((".eslintrc.json", ".prettierrc.json", ".babelrc.json",
|
||||
"tsconfig.json", "jsconfig.json")):
|
||||
return True
|
||||
# Top-level key probe: scan the root object's immediate keys (no deep walk).
|
||||
for top_key in obj_node.children:
|
||||
if top_key.type != "pair":
|
||||
continue
|
||||
key_node = top_key.child_by_field_name("key")
|
||||
if key_node is None:
|
||||
continue
|
||||
kc = key_node.child_by_field_name("string_content")
|
||||
text = _read_text(kc, source) if kc else _read_text(key_node, source).strip('"\'')
|
||||
if text in _CONFIG_JSON_KEYS:
|
||||
return True
|
||||
return False
|
||||
|
||||
|
||||
def extract_json(path: Path) -> dict:
|
||||
"""Extract top-level keys, nested structure, and dependency edges from a .json file."""
|
||||
"""Extract structure and dependency edges from a *config/manifest* .json file.
|
||||
|
||||
Data-shaped JSON (eval fixtures, datasets, GeoJSON, API response dumps) is
|
||||
deliberately skipped — AST-walking it produced hundreds of orphan key-nodes
|
||||
and duplicate communities that swamped real structure (#1224). Recognition
|
||||
is by filename (package.json, tsconfig.json, …) or a top-level key probe
|
||||
(dependencies / extends / $ref / $schema / compilerOptions)."""
|
||||
_JSON_MAX_BYTES = 1_048_576 # 1 MiB — skip large fixture dumps / GeoJSON blobs
|
||||
|
||||
try:
|
||||
@@ -10342,7 +10398,15 @@ def extract_json(path: Path) -> dict:
|
||||
if doc.type == "document" and doc.child_count > 0:
|
||||
doc = doc.children[0]
|
||||
if doc.type == "object":
|
||||
# Only AST-extract recognized config/manifest JSON. Data JSON (fixtures,
|
||||
# datasets, GeoJSON, API dumps) is skipped so it doesn't explode into
|
||||
# orphan key-nodes (#1224); it's left to the LLM semantic pass.
|
||||
if not _is_config_json(path, doc, source):
|
||||
return {"nodes": [], "edges": [], "skipped": "data json (not a config/manifest)"}
|
||||
walk_object(doc, file_nid, None, 0, [0])
|
||||
else:
|
||||
# Top-level array or scalar => data JSON, never a config/manifest.
|
||||
return {"nodes": [], "edges": [], "skipped": "data json (non-object root)"}
|
||||
|
||||
return {"nodes": nodes, "edges": edges}
|
||||
|
||||
|
||||
+92
-11
@@ -95,6 +95,10 @@ BACKENDS: dict[str, dict] = {
|
||||
"env_key": "OPENAI_API_KEY",
|
||||
"model_env_key": "GRAPHIFY_OPENAI_MODEL",
|
||||
"pricing": {"input": 0.40, "output": 1.60}, # USD per 1M tokens
|
||||
# Default (gpt-4.1-mini) accepts temperature=0. Reasoning models
|
||||
# (o1/o3/o4/gpt-5) reject any explicit temperature and have it omitted
|
||||
# automatically by _resolve_temperature; GRAPHIFY_LLM_TEMPERATURE
|
||||
# overrides either way (#1191).
|
||||
"temperature": 0,
|
||||
"vision": True,
|
||||
},
|
||||
@@ -244,6 +248,80 @@ def _resolve_max_tokens(default: int) -> int:
|
||||
return default
|
||||
|
||||
|
||||
# Model-name fragments for OpenAI-compatible "reasoning" models that reject an
|
||||
# explicit temperature: the API returns 400 "Unsupported value: 'temperature'
|
||||
# does not support 0 with this model. Only the default (1) value is supported."
|
||||
# Covers the o1/o3/o4 reasoning series and the gpt-5 family, which share the
|
||||
# same restriction. Matched case-insensitively against the resolved model id
|
||||
# (issue #1191).
|
||||
_FIXED_TEMPERATURE_MODEL_MARKERS = ("o1", "o1-", "o3", "o3-", "o4", "o4-", "gpt-5")
|
||||
|
||||
|
||||
def _model_requires_default_temperature(model: str) -> bool:
|
||||
"""True if `model` is a reasoning model that rejects an explicit temperature.
|
||||
|
||||
OpenAI's o-series (o1, o3, o4...) and gpt-5 family only accept the default
|
||||
temperature (1) and return HTTP 400 if any value — including 0 — is sent.
|
||||
We must omit the parameter entirely for these (#1191).
|
||||
"""
|
||||
m = (model or "").lower()
|
||||
# Strip a leading "openai/" or provider prefix some gateways prepend.
|
||||
base = m.rsplit("/", 1)[-1]
|
||||
if base.startswith("gpt-5"):
|
||||
return True
|
||||
# o1 / o3 / o4 family: bare ("o1") or versioned ("o3-mini", "o1-preview").
|
||||
for fam in ("o1", "o3", "o4"):
|
||||
if base == fam or base.startswith(fam + "-"):
|
||||
return True
|
||||
return False
|
||||
|
||||
|
||||
def _resolve_temperature(default: float | None, model: str = "") -> float | None:
|
||||
"""Resolve the temperature to send, honouring GRAPHIFY_LLM_TEMPERATURE.
|
||||
|
||||
Precedence (issue #1191):
|
||||
1. GRAPHIFY_LLM_TEMPERATURE env var, if set:
|
||||
- a numeric value (e.g. "0", "0.2", "1") is used verbatim;
|
||||
- the literal "none"/"omit"/"default" (case-insensitive) means
|
||||
"omit the temperature parameter entirely" (-> None).
|
||||
2. Otherwise, reasoning models (o1/o3/o4/gpt-5) get None — the parameter
|
||||
must be omitted or the API rejects the request.
|
||||
3. Otherwise, the backend config default (`default`, usually 0).
|
||||
|
||||
Returns None when the temperature parameter should be omitted from the
|
||||
request; the call sites already guard `if temperature is not None`.
|
||||
"""
|
||||
raw = os.environ.get("GRAPHIFY_LLM_TEMPERATURE", "").strip()
|
||||
if raw:
|
||||
if raw.lower() in ("none", "omit", "default"):
|
||||
return None
|
||||
try:
|
||||
return float(raw)
|
||||
except ValueError:
|
||||
print(
|
||||
f"[graphify] GRAPHIFY_LLM_TEMPERATURE={raw!r} is not a number or "
|
||||
"'none'; falling back to the backend default.",
|
||||
file=sys.stderr,
|
||||
)
|
||||
if _model_requires_default_temperature(model):
|
||||
return None
|
||||
return default
|
||||
|
||||
|
||||
def _bedrock_inference_config(max_tokens: int, model: str = "") -> dict:
|
||||
"""Build Bedrock inferenceConfig, honouring GRAPHIFY_LLM_TEMPERATURE.
|
||||
|
||||
Bedrock's Converse API treats `temperature` as optional; omitting it uses
|
||||
the model default. We default to 0 for deterministic extraction but let the
|
||||
env var override (or omit) it for parity with the OpenAI-compatible path.
|
||||
"""
|
||||
cfg: dict = {"maxTokens": max_tokens}
|
||||
temp = _resolve_temperature(0, model)
|
||||
if temp is not None:
|
||||
cfg["temperature"] = temp
|
||||
return cfg
|
||||
|
||||
|
||||
def _resolve_api_timeout(default: float = 600.0) -> float:
|
||||
"""Honour GRAPHIFY_API_TIMEOUT env var override, else use default (seconds)."""
|
||||
raw = os.environ.get("GRAPHIFY_API_TIMEOUT", "").strip()
|
||||
@@ -1100,7 +1178,7 @@ def _call_bedrock(model: str, user_message: str, max_tokens: int = 8192, *, deep
|
||||
modelId=model,
|
||||
system=[{"text": _extraction_system(deep=deep_mode)}],
|
||||
messages=[{"role": "user", "content": _bedrock_content(user_message, images or [])}],
|
||||
inferenceConfig={"maxTokens": max_tokens, "temperature": 0},
|
||||
inferenceConfig=_bedrock_inference_config(max_tokens, model),
|
||||
)
|
||||
except botocore.exceptions.ClientError as exc:
|
||||
code = exc.response["Error"]["Code"]
|
||||
@@ -1204,7 +1282,7 @@ def extract_files_direct(
|
||||
endpoint,
|
||||
mdl,
|
||||
user_msg,
|
||||
temperature=cfg.get("temperature", 0),
|
||||
temperature=_resolve_temperature(cfg.get("temperature", 0), mdl),
|
||||
max_tokens=max_out,
|
||||
deep_mode=deep_mode,
|
||||
)
|
||||
@@ -1213,7 +1291,7 @@ def extract_files_direct(
|
||||
key,
|
||||
mdl,
|
||||
user_msg,
|
||||
temperature=cfg.get("temperature", 0),
|
||||
temperature=_resolve_temperature(cfg.get("temperature", 0), mdl),
|
||||
reasoning_effort=cfg.get("reasoning_effort"),
|
||||
max_completion_tokens=_resolve_max_tokens(cfg.get("max_completion_tokens", 8192)),
|
||||
backend=backend,
|
||||
@@ -1668,7 +1746,7 @@ def _call_llm(prompt: str, *, backend: str, max_tokens: int = 200) -> str:
|
||||
resp = client.converse(
|
||||
modelId=mdl,
|
||||
messages=[{"role": "user", "content": [{"text": prompt}]}],
|
||||
inferenceConfig={"maxTokens": max_tokens, "temperature": 0},
|
||||
inferenceConfig=_bedrock_inference_config(max_tokens, mdl),
|
||||
)
|
||||
return resp.get("output", {}).get("message", {}).get("content", [{}])[0].get("text", "")
|
||||
|
||||
@@ -1679,12 +1757,15 @@ def _call_llm(prompt: str, *, backend: str, max_tokens: int = 200) -> str:
|
||||
"Azure OpenAI backend requires AZURE_OPENAI_ENDPOINT to be set."
|
||||
)
|
||||
azure_client = _azure_client(key, endpoint)
|
||||
resp = azure_client.chat.completions.create(
|
||||
model=mdl,
|
||||
messages=[{"role": "user", "content": prompt}],
|
||||
max_completion_tokens=max_tokens,
|
||||
temperature=cfg.get("temperature", 0),
|
||||
)
|
||||
azure_kwargs: dict = {
|
||||
"model": mdl,
|
||||
"messages": [{"role": "user", "content": prompt}],
|
||||
"max_completion_tokens": max_tokens,
|
||||
}
|
||||
azure_temp = _resolve_temperature(cfg.get("temperature", 0), mdl)
|
||||
if azure_temp is not None:
|
||||
azure_kwargs["temperature"] = azure_temp
|
||||
resp = azure_client.chat.completions.create(**azure_kwargs)
|
||||
if not resp.choices or resp.choices[0].message is None:
|
||||
raise ValueError("Azure OpenAI returned empty or filtered response")
|
||||
return resp.choices[0].message.content or ""
|
||||
@@ -1700,7 +1781,7 @@ def _call_llm(prompt: str, *, backend: str, max_tokens: int = 200) -> str:
|
||||
"messages": [{"role": "user", "content": prompt}],
|
||||
"max_completion_tokens": max_tokens,
|
||||
}
|
||||
temperature = cfg.get("temperature", 0)
|
||||
temperature = _resolve_temperature(cfg.get("temperature", 0), mdl)
|
||||
if temperature is not None:
|
||||
kwargs["temperature"] = temperature
|
||||
if cfg.get("reasoning_effort"):
|
||||
|
||||
@@ -1,5 +1,7 @@
|
||||
import unicodedata
|
||||
from pathlib import Path
|
||||
from graphify.detect import classify_file, count_words, detect, detect_incremental, save_manifest, FileType, _looks_like_paper, _is_ignored, _load_graphifyignore, _is_sensitive
|
||||
from graphify import detect as detect_mod
|
||||
|
||||
FIXTURES = Path(__file__).parent / "fixtures"
|
||||
|
||||
@@ -1395,3 +1397,49 @@ def test_save_manifest_in_root_symlink_roundtrips(tmp_path):
|
||||
|
||||
loaded = load_manifest(manifest_path, root=tmp_path)
|
||||
assert str(tmp_path.resolve() / "alias.py") in loaded
|
||||
|
||||
|
||||
def test_convert_office_file_hash_stable_across_nfc_nfd(tmp_path, monkeypatch):
|
||||
"""The sidecar name must be identical whether the source path arrives in
|
||||
NFC or NFD form. On macOS os.walk/rglob yield NFD paths while directly
|
||||
constructed Paths are NFC; without NFC-normalizing before hashing the same
|
||||
.docx would get a different sidecar name (and manifest key) on every run,
|
||||
forcing a full re-extraction under --update (#1226).
|
||||
"""
|
||||
monkeypatch.setattr(detect_mod, "docx_to_markdown", lambda p: "hello world")
|
||||
|
||||
out_dir = tmp_path / "converted"
|
||||
# "한글" / "ä" style filename with a precomposed (NFC) and decomposed (NFD)
|
||||
# representation that are distinct byte strings but the same logical name.
|
||||
base = tmp_path / "report"
|
||||
nfc_name = unicodedata.normalize("NFC", "café.docx")
|
||||
nfd_name = unicodedata.normalize("NFD", "café.docx")
|
||||
assert nfc_name != nfd_name # sanity: the two forms differ byte-wise
|
||||
|
||||
nfc_path = base / nfc_name
|
||||
nfd_path = base / nfd_name
|
||||
|
||||
out_nfc = detect_mod.convert_office_file(nfc_path, out_dir)
|
||||
out_nfd = detect_mod.convert_office_file(nfd_path, out_dir)
|
||||
|
||||
assert out_nfc is not None and out_nfd is not None
|
||||
# The hash suffix (and therefore the whole sidecar filename) must match.
|
||||
assert out_nfc.name.split("_")[-1] == out_nfd.name.split("_")[-1]
|
||||
|
||||
|
||||
def test_convert_office_file_does_not_rewrite_existing_sidecar(tmp_path, monkeypatch):
|
||||
"""A second conversion of an unchanged source must not rewrite the sidecar,
|
||||
so its mtime stays put and detect_incremental keeps treating it as
|
||||
unchanged (#1226)."""
|
||||
monkeypatch.setattr(detect_mod, "docx_to_markdown", lambda p: "hello world")
|
||||
|
||||
out_dir = tmp_path / "converted"
|
||||
src = tmp_path / "doc.docx"
|
||||
|
||||
first = detect_mod.convert_office_file(src, out_dir)
|
||||
assert first is not None
|
||||
mtime_before = first.stat().st_mtime_ns
|
||||
|
||||
second = detect_mod.convert_office_file(src, out_dir)
|
||||
assert second == first
|
||||
assert second.stat().st_mtime_ns == mtime_before
|
||||
|
||||
@@ -1,3 +1,4 @@
|
||||
import json
|
||||
from pathlib import Path
|
||||
from graphify.extract import extract_python, extract, collect_files, _make_id, extract_bash, extract_json, _DISPATCH
|
||||
|
||||
@@ -905,6 +906,52 @@ def test_extract_json_no_self_loops():
|
||||
assert e["source"] != e["target"], f"Self-loop: {e}"
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# Data JSON must not explode into orphan key-nodes (#1224)
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
def test_extract_json_data_file_skipped(tmp_path):
|
||||
"""A data-shaped .json (eval fixture / dataset) must NOT emit per-key nodes."""
|
||||
data = tmp_path / "cases.json"
|
||||
data.write_text(json.dumps({
|
||||
"generation": {"target": "gpt-4", "cases_file": "c.json", "num_cases": 12},
|
||||
"prompt_inputs_spec": {"a": 1, "b": 2},
|
||||
"suite": [{"name": "x"}, {"name": "y"}],
|
||||
}))
|
||||
result = extract_json(data)
|
||||
assert result["nodes"] == []
|
||||
assert result["edges"] == []
|
||||
assert "skipped" in result
|
||||
|
||||
|
||||
def test_extract_json_top_level_array_skipped(tmp_path):
|
||||
"""A JSON file whose root is an array is data, never a config/manifest."""
|
||||
data = tmp_path / "records.json"
|
||||
data.write_text(json.dumps([{"id": 1}, {"id": 2}]))
|
||||
result = extract_json(data)
|
||||
assert result["nodes"] == []
|
||||
assert result["edges"] == []
|
||||
|
||||
|
||||
def test_extract_json_config_by_filename_still_extracted(tmp_path):
|
||||
"""tsconfig.json must still be AST-extracted even without telltale keys."""
|
||||
cfg = tmp_path / "tsconfig.json"
|
||||
cfg.write_text(json.dumps({"compilerOptions": {"strict": True}}))
|
||||
result = extract_json(cfg)
|
||||
assert len(result["nodes"]) > 0
|
||||
assert "skipped" not in result
|
||||
|
||||
|
||||
def test_extract_json_config_by_key_probe(tmp_path):
|
||||
"""An arbitrarily-named JSON with config keys (dependencies) is still extracted."""
|
||||
cfg = tmp_path / "weird-name.json"
|
||||
cfg.write_text(json.dumps({"dependencies": {"lodash": "^4"}}))
|
||||
result = extract_json(cfg)
|
||||
import_edges = [e for e in result["edges"] if e["relation"] == "imports"]
|
||||
assert any("lodash" in e["target"] for e in import_edges)
|
||||
assert "skipped" not in result
|
||||
|
||||
|
||||
def test_extract_bash_via_dispatch():
|
||||
from graphify.extract import _get_extractor
|
||||
assert _get_extractor(Path("foo.sh")) is extract_bash
|
||||
|
||||
@@ -650,3 +650,99 @@ def test_detect_backend_azure_requires_endpoint_not_just_key(monkeypatch):
|
||||
def test_estimate_cost_azure_no_keyerror():
|
||||
cost = llm.estimate_cost("azure", 1_000_000, 500_000)
|
||||
assert cost == pytest.approx(2.50 + 5.00) # 1M in * $2.50/M + 0.5M out * $10.00/M
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# Temperature resolution (#1191): omit temperature for reasoning models
|
||||
# (o1/o3/o4/gpt-5) and honour GRAPHIFY_LLM_TEMPERATURE.
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
|
||||
@pytest.mark.parametrize(
|
||||
"model",
|
||||
["o1", "o1-preview", "o1-mini", "o3", "o3-mini", "o4-mini", "gpt-5", "gpt-5-mini", "openai/o3-mini"],
|
||||
)
|
||||
def test_model_requires_default_temperature_true_for_reasoning_models(model):
|
||||
assert llm._model_requires_default_temperature(model) is True
|
||||
|
||||
|
||||
@pytest.mark.parametrize(
|
||||
"model",
|
||||
["gpt-4.1-mini", "gpt-4o", "gpt-4.1", "kimi-k2.6", "deepseek-v4-flash", "", "o1x", "go3"],
|
||||
)
|
||||
def test_model_requires_default_temperature_false_for_normal_models(model):
|
||||
assert llm._model_requires_default_temperature(model) is False
|
||||
|
||||
|
||||
def test_resolve_temperature_default_for_normal_model(monkeypatch):
|
||||
monkeypatch.delenv("GRAPHIFY_LLM_TEMPERATURE", raising=False)
|
||||
assert llm._resolve_temperature(0, "gpt-4.1-mini") == 0
|
||||
|
||||
|
||||
def test_resolve_temperature_omitted_for_reasoning_model(monkeypatch):
|
||||
monkeypatch.delenv("GRAPHIFY_LLM_TEMPERATURE", raising=False)
|
||||
assert llm._resolve_temperature(0, "o3-mini") is None
|
||||
assert llm._resolve_temperature(0, "gpt-5") is None
|
||||
|
||||
|
||||
def test_resolve_temperature_env_var_numeric_overrides(monkeypatch):
|
||||
monkeypatch.setenv("GRAPHIFY_LLM_TEMPERATURE", "0.7")
|
||||
assert llm._resolve_temperature(0, "gpt-4.1-mini") == 0.7
|
||||
# env var wins even for a reasoning model (explicit user choice)
|
||||
assert llm._resolve_temperature(0, "o3-mini") == 0.7
|
||||
|
||||
|
||||
def test_resolve_temperature_env_var_none_omits(monkeypatch):
|
||||
monkeypatch.setenv("GRAPHIFY_LLM_TEMPERATURE", "none")
|
||||
assert llm._resolve_temperature(0, "gpt-4.1-mini") is None
|
||||
|
||||
|
||||
def test_resolve_temperature_env_var_invalid_falls_back(monkeypatch):
|
||||
monkeypatch.setenv("GRAPHIFY_LLM_TEMPERATURE", "hot")
|
||||
# bad value -> backend default for a normal model, still omitted for reasoning
|
||||
assert llm._resolve_temperature(0, "gpt-4.1-mini") == 0
|
||||
assert llm._resolve_temperature(0, "o3-mini") is None
|
||||
|
||||
|
||||
def test_openai_compat_omits_temperature_for_o3_model(tmp_path, monkeypatch):
|
||||
# Regression for #1191: with a reasoning model the request must not carry a
|
||||
# `temperature` key at all, or the API returns HTTP 400.
|
||||
_clear_backend_env(monkeypatch)
|
||||
monkeypatch.delenv("GRAPHIFY_LLM_TEMPERATURE", raising=False)
|
||||
monkeypatch.setenv("OPENAI_API_KEY", "sk-test")
|
||||
monkeypatch.setenv("GRAPHIFY_OPENAI_MODEL", "o3-mini")
|
||||
captured = _install_capturing_openai(monkeypatch)
|
||||
(tmp_path / "f.py").write_text("x = 1\n")
|
||||
|
||||
llm.extract_files_direct([tmp_path / "f.py"], backend="openai", root=tmp_path)
|
||||
|
||||
assert "temperature" not in captured, (
|
||||
"reasoning models (o3) reject an explicit temperature; it must be omitted (#1191)"
|
||||
)
|
||||
assert captured["model"] == "o3-mini"
|
||||
|
||||
|
||||
def test_openai_compat_sends_temperature_for_normal_model(tmp_path, monkeypatch):
|
||||
_clear_backend_env(monkeypatch)
|
||||
monkeypatch.delenv("GRAPHIFY_LLM_TEMPERATURE", raising=False)
|
||||
monkeypatch.delenv("GRAPHIFY_OPENAI_MODEL", raising=False)
|
||||
monkeypatch.setenv("OPENAI_API_KEY", "sk-test")
|
||||
captured = _install_capturing_openai(monkeypatch)
|
||||
(tmp_path / "f.py").write_text("x = 1\n")
|
||||
|
||||
llm.extract_files_direct([tmp_path / "f.py"], backend="openai", root=tmp_path)
|
||||
|
||||
assert captured.get("temperature") == 0, "normal models keep the deterministic default"
|
||||
|
||||
|
||||
def test_openai_compat_env_var_temperature_applied(tmp_path, monkeypatch):
|
||||
_clear_backend_env(monkeypatch)
|
||||
monkeypatch.setenv("GRAPHIFY_LLM_TEMPERATURE", "0.3")
|
||||
monkeypatch.delenv("GRAPHIFY_OPENAI_MODEL", raising=False)
|
||||
monkeypatch.setenv("OPENAI_API_KEY", "sk-test")
|
||||
captured = _install_capturing_openai(monkeypatch)
|
||||
(tmp_path / "f.py").write_text("x = 1\n")
|
||||
|
||||
llm.extract_files_direct([tmp_path / "f.py"], backend="openai", root=tmp_path)
|
||||
|
||||
assert captured.get("temperature") == 0.3
|
||||
|
||||
@@ -0,0 +1,50 @@
|
||||
"""Regression test for issue #1236: to_obsidian must not crash with KeyError
|
||||
when a community's member list contains an id that has no backing node in G
|
||||
(e.g. pruned nodes, stale community assignments, or synthesized/merge-artifact
|
||||
ids). Such dangling members must be skipped, not abort the whole vault export."""
|
||||
import networkx as nx
|
||||
|
||||
from graphify.export import to_obsidian
|
||||
|
||||
|
||||
def _graph_with_dangling_member():
|
||||
"""Two real nodes plus a community that references a third, non-existent id."""
|
||||
G = nx.Graph()
|
||||
G.add_node("n0", label="Alpha", file_type="code", source_file="a.py")
|
||||
G.add_node("n1", label="Beta", file_type="code", source_file="b.py")
|
||||
G.add_edge("n0", "n1", relation="calls", confidence="EXTRACTED")
|
||||
# 'agents_doc' is a synthesized member id with no backing node in G.
|
||||
communities = {0: ["n0", "n1", "agents_doc"]}
|
||||
return G, communities
|
||||
|
||||
|
||||
def test_obsidian_dangling_community_member_does_not_crash(tmp_path):
|
||||
G, comms = _graph_with_dangling_member()
|
||||
# Before the fix this raised KeyError: 'agents_doc'.
|
||||
n = to_obsidian(G, comms, str(tmp_path))
|
||||
assert n > 0
|
||||
|
||||
# The community note is still written for the surviving members.
|
||||
comm_notes = list(tmp_path.glob("_COMMUNITY_*.md"))
|
||||
assert len(comm_notes) == 1
|
||||
body = comm_notes[0].read_text(encoding="utf-8")
|
||||
|
||||
# Real members appear in the Members section; the dangling id does not.
|
||||
assert "[[Alpha]]" in body
|
||||
assert "[[Beta]]" in body
|
||||
assert "agents_doc" not in body
|
||||
|
||||
# Member count reflects only the real (resolvable) members.
|
||||
assert "**Members:** 2 nodes" in body
|
||||
|
||||
|
||||
def test_obsidian_community_of_only_dangling_members(tmp_path):
|
||||
"""A community whose members are all dangling should still not crash."""
|
||||
G = nx.Graph()
|
||||
G.add_node("n0", label="Alpha", file_type="code", source_file="a.py")
|
||||
comms = {0: ["n0"], 1: ["ghost_a", "ghost_b"]}
|
||||
n = to_obsidian(G, comms, str(tmp_path))
|
||||
assert n > 0
|
||||
ghost_note = tmp_path / "_COMMUNITY_Community 1.md"
|
||||
assert ghost_note.exists()
|
||||
assert "**Members:** 0 nodes" in ghost_note.read_text(encoding="utf-8")
|
||||
Reference in New Issue
Block a user