From cdc920e9c8d2ad6c1321bdf3bea1d31d208b75b9 Mon Sep 17 00:00:00 2001 From: Lukas Gold Date: Thu, 17 Sep 2026 16:16:37 +0200 Subject: [PATCH 01/17] feat(service): add task and project operations with a Claude skill - seven task operations shared by the CLI and the MCP server - fixed status and priority vocabularies; references resolved by label - OSW_PERSON_IRI plus three category overrides, all optional - osl-tasks skill, installed by `osw skill install` or as a plugin - reads the stored page uncached, and verifies a write really landed --- .claude-plugin/marketplace.json | 15 + .claude-plugin/plugin.json | 14 + docs/tools/cli.md | 45 ++ docs/tools/configuration.md | 4 + src/osw/cli/main.py | 2 + src/osw/cli/ops.py | 39 ++ src/osw/service/config.py | 33 ++ src/osw/service/ops/__init__.py | 2 +- src/osw/service/ops/tasks.py | 779 ++++++++++++++++++++++++++++++ src/osw/skills/osl-tasks/SKILL.md | 171 +++++++ tests/test_cli_ops_skill.py | 77 +++ tests/test_service_ops_tasks.py | 559 +++++++++++++++++++++ 12 files changed, 1739 insertions(+), 1 deletion(-) create mode 100644 .claude-plugin/marketplace.json create mode 100644 .claude-plugin/plugin.json create mode 100644 src/osw/service/ops/tasks.py create mode 100644 src/osw/skills/osl-tasks/SKILL.md create mode 100644 tests/test_cli_ops_skill.py create mode 100644 tests/test_service_ops_tasks.py diff --git a/.claude-plugin/marketplace.json b/.claude-plugin/marketplace.json new file mode 100644 index 00000000..d2359507 --- /dev/null +++ b/.claude-plugin/marketplace.json @@ -0,0 +1,15 @@ +{ + "name": "osw-python", + "description": "Skills for working with OpenSemanticLab wikis through the osw CLI and MCP server.", + "owner": { + "name": "OpenSemanticLab", + "url": "https://github.com/OpenSemanticLab" + }, + "plugins": [ + { + "name": "osl-tasks", + "source": "./", + "description": "Task and project management for OpenSemanticLab, driven by the osw CLI and MCP server." + } + ] +} diff --git a/.claude-plugin/plugin.json b/.claude-plugin/plugin.json new file mode 100644 index 00000000..302663f9 --- /dev/null +++ b/.claude-plugin/plugin.json @@ -0,0 +1,14 @@ +{ + "name": "osl-tasks", + "displayName": "OSL task management", + "description": "Task and project management for OpenSemanticLab, driven by the osw CLI and MCP server.", + "version": "0.1.0", + "author": { + "name": "OpenSemanticLab" + }, + "homepage": "https://github.com/OpenSemanticLab/osw-python", + "repository": "https://github.com/OpenSemanticLab/osw-python", + "license": "Apache-2.0", + "keywords": ["opensemanticlab", "osl", "tasks", "wiki", "semantic-mediawiki"], + "skills": ["./src/osw/skills/"] +} diff --git a/docs/tools/cli.md b/docs/tools/cli.md index c357787f..f53f1859 100644 --- a/docs/tools/cli.md +++ b/docs/tools/cli.md @@ -37,6 +37,8 @@ Commands are grouped by subject: | `search` | `ask`, `titles`, `content`, `entities`, `sparql` | | `slot` | `list`, `get`, `set` | | `schema` | `get` | +| `task` | `create`, `update`, `list`, `list-projects`, `list-persons`, `create-person`, `render` | +| `skill` | `install` | | `instances` | `list`, `status` | | `ledger` | `path` | | top level | `status` | @@ -79,3 +81,46 @@ form. help. Failures exit non-zero with a short message on stderr and no traceback. + +## Tasks and projects + +The `task` group reads local todos into an OSL wiki as Task entities, and +reads tasks, projects and persons back out. It is built on three OSL core +categories (Task, Person, Project); every operation takes plain typed +parameters and returns a small flat dict, never a JSON Schema. + +| Command | Tool | Purpose | +| --- | --- | --- | +| `osw task create` | `create_task` | Create a task. | +| `osw task update` | `update_task` | Merge fields into an existing task. | +| `osw task list` | `list_tasks` | List tasks, filtered by project, actionee, status or label text. | +| `osw task list-projects` | `list_projects` | Find a project's page name. | +| `osw task list-persons` | `list_persons` | Find a person's page name. | +| `osw task create-person` | `create_person` | Create a person, as a fallback for when one is genuinely absent. | +| `osw task render` | not available | Render a Markdown table of tasks to a local file. | + +`render` is CLI only: it names a local output path, and no MCP tool takes or +returns a path. + +**Configuration.** Four environment variables affect these operations, and +all are optional: `OSW_PERSON_IRI`, `OSW_TASK_CATEGORY`, `OSW_PERSON_CATEGORY` +and `OSW_PROJECT_CATEGORY`. Reading always queries the shared OSL core +category, since MediaWiki category membership includes the whole subclass +tree, so a task kept in a local subclass is found without any configuration. +The three category overrides only change where a newly created task, person +or project is written. + +**Vocabularies.** `status` is one of `to do`, `in work`, `done`. `prio` is one +of `high`, `medium`, `low`. A due date is written to `end_date_time`, since +the Task category has no due-date property. + +### The Claude Code skill + +The skill that drives this group ships at `src/osw/skills/osl-tasks/SKILL.md`. +Install it one of two ways: + +1. `osw skill install`, which copies it to `~/.claude/skills/osl-tasks/`. +2. `/plugin marketplace add OpenSemanticLab/osw-python` then + `/plugin install osl-tasks`. + +A new Claude Code session picks it up with no further action. diff --git a/docs/tools/configuration.md b/docs/tools/configuration.md index fad7ca7b..5bc10e70 100644 --- a/docs/tools/configuration.md +++ b/docs/tools/configuration.md @@ -111,6 +111,10 @@ set wins: | `OSW_MAX_RESULTS` | `OSW_MCP_MAX_RESULTS` | Default result cap (100) | | `OSW_MAX_CHARS` | `OSW_MCP_MAX_CHARS` | Result size cap in characters (100000) | | `OSW_VERBOSE` | `OSW_MCP_VERBOSE` | `true` prints the configuration source report | +| `OSW_PERSON_IRI` | | Page name of the operator's own Person entity, used by `list_tasks(mine=True)` | +| `OSW_TASK_CATEGORY` | | Category a newly created task is written to | +| `OSW_PERSON_CATEGORY` | | Category a newly created person is written to | +| `OSW_PROJECT_CATEGORY` | | Category used when resolving a project by name | ## Windows paths in a `.env` file diff --git a/src/osw/cli/main.py b/src/osw/cli/main.py index 7eee7a90..7b6cb769 100644 --- a/src/osw/cli/main.py +++ b/src/osw/cli/main.py @@ -323,7 +323,9 @@ def command(**kwargs: Any) -> None: "schema": "Category JSON Schemas.", "search": "Find pages. OSW pages are titled by OSW-ID, so use 'ask' " "to search by name.", + "skill": "Install the Claude Code skills that ship with this package.", "slot": "Read and write individual page slots.", + "task": "Create, find and update tasks, projects and persons.", } for _op in iter_operations(surface="cli"): diff --git a/src/osw/cli/ops.py b/src/osw/cli/ops.py index 0b7d502e..62cf9669 100644 --- a/src/osw/cli/ops.py +++ b/src/osw/cli/ops.py @@ -148,6 +148,45 @@ def ledger_path(ctx: Context) -> dict: return {"path": str(ctx.ledger.path)} +@operation( + group="skill", + cli_name="install", + surfaces=frozenset({"cli"}), + idempotent_hint=True, +) +def install_skill( + ctx: Context, + name: str = "osl-tasks", + target_dir: Optional[str] = None, + force: bool = False, +) -> dict: + """Install a Claude Code skill that ships with this package. + + Copies the packaged skill directory ``src/osw/skills/`` to + ``~/.claude/skills/`` (or under ``target_dir`` when given). A new + Claude Code session picks the installed skill up automatically, with no + further action needed. This is the alternative to installing the + osw-python plugin from its marketplace. + """ + src = Path(__file__).resolve().parent.parent / "skills" / name + if not src.is_dir(): + available = sorted(p.name for p in src.parent.iterdir() if p.is_dir()) + raise errors.NotFound( + f"No packaged skill named '{name}'. Available: {', '.join(available)}." + ) + + base = Path(target_dir) if target_dir else Path.home() / ".claude" / "skills" + dest = base / name + if dest.exists() and not force: + raise errors.OpError(f"'{dest}' already exists. Pass --force to overwrite it.") + + shutil.copytree(src, dest, dirs_exist_ok=True) + files = sorted( + p.relative_to(dest).as_posix() for p in dest.rglob("*") if p.is_file() + ) + return {"name": name, "source": str(src), "target": str(dest), "files": files} + + @operation( group="instances", cli_name="list", diff --git a/src/osw/service/config.py b/src/osw/service/config.py index c68e6649..d868a391 100644 --- a/src/osw/service/config.py +++ b/src/osw/service/config.py @@ -40,6 +40,10 @@ ENV_MAX_CHARS = ("OSW_MAX_CHARS", "OSW_MCP_MAX_CHARS") ENV_FILE = ("OSW_ENV_FILE", "OSW_MCP_ENV_FILE") ENV_VERBOSE = ("OSW_VERBOSE", "OSW_MCP_VERBOSE") +ENV_TASK_CATEGORY = ("OSW_TASK_CATEGORY",) +ENV_PERSON_CATEGORY = ("OSW_PERSON_CATEGORY",) +ENV_PROJECT_CATEGORY = ("OSW_PROJECT_CATEGORY",) +ENV_PERSON_IRI = ("OSW_PERSON_IRI",) def _first_env(names: tuple[str, ...]) -> Optional[str]: @@ -64,6 +68,10 @@ def _first_env(names: tuple[str, ...]) -> Optional[str]: "max_results": ENV_MAX_RESULTS, "max_chars": ENV_MAX_CHARS, "verbose": ENV_VERBOSE, + "task_category": ENV_TASK_CATEGORY, + "person_category": ENV_PERSON_CATEGORY, + "project_category": ENV_PROJECT_CATEGORY, + "person_iri": ENV_PERSON_IRI, } @@ -105,6 +113,16 @@ class Settings(BaseModel): # Only controls the startup configuration report. No tool or command # reads it, and Settings.redacted() deliberately does not expose it. verbose: bool = False + # Category a newly created task/person/project gets; defaults to the + # matching core constant in osw.service.ops.tasks when unset. + task_category: Optional[str] = None + person_category: Optional[str] = None + project_category: Optional[str] = None + # Full page name of the Person entity representing the operator, e.g. + # "Item:OSW...". Read only by list_tasks(mine=True) in + # osw.service.ops.tasks. An actionee is never assigned from it, so a task + # created without an explicit actionee has none. + person_iri: Optional[str] = None @field_validator("domain") @classmethod @@ -166,6 +184,17 @@ def _validate_state_dir(cls, value: Optional[str]) -> Optional[str]: ) return value + @field_validator( + "task_category", "person_category", "project_category", "person_iri" + ) + @classmethod + def _validate_non_blank(cls, value: Optional[str]) -> Optional[str]: + if value is None: + return value + if not value.strip(): + raise ValueError("must not be empty or whitespace-only") + return value + @field_validator("cred_filepath") @classmethod def _validate_cred_filepath(cls, value: Optional[str]) -> Optional[str]: @@ -755,6 +784,10 @@ def load(strict: bool = True) -> Settings: cred_filepath=cred_filepath, sparql_endpoint=_first_env(ENV_SPARQL_ENDPOINT), state_dir=_first_env(ENV_STATE_DIR), + task_category=_first_env(ENV_TASK_CATEGORY), + person_category=_first_env(ENV_PERSON_CATEGORY), + project_category=_first_env(ENV_PROJECT_CATEGORY), + person_iri=_first_env(ENV_PERSON_IRI), ) # An unset or blank/whitespace-only variable falls back to the model # default; pass the raw string only when there is one to validate. Letting diff --git a/src/osw/service/ops/__init__.py b/src/osw/service/ops/__init__.py index 9c5c6444..68d5a381 100644 --- a/src/osw/service/ops/__init__.py +++ b/src/osw/service/ops/__init__.py @@ -15,4 +15,4 @@ from __future__ import annotations -from . import entities, files, schema, search, slots, status +from . import entities, files, schema, search, slots, status, tasks diff --git a/src/osw/service/ops/tasks.py b/src/osw/service/ops/tasks.py new file mode 100644 index 00000000..d1ebc52e --- /dev/null +++ b/src/osw/service/ops/tasks.py @@ -0,0 +1,779 @@ +"""Task management operations: create/update/list tasks, projects, persons. + +Narrow, field-based operations for OSL task management, built on three OSL +core categories (Task, Person, Project). Every operation takes plain typed +parameters and returns a small flat dict, so an agent driving this spends few +tokens; no JSON Schema is ever handed to the caller. +""" + +from __future__ import annotations + +import re +from datetime import datetime +from typing import Optional +from uuid import uuid4 + +from osw.core import OSW, OverwriteOptions +from osw.service import config, errors +from osw.service.context import Context +from osw.service.ledger import LedgerRecord +from osw.service.ops.entities import _resolve_category_class +from osw.service.registry import operation +from osw.wtsite import WtSite + +CATEGORY_TASK = "Category:OSWc5d4829ed2744a219ba027171c75fa1d" +CATEGORY_PERSON = "Category:OSW44deaa5b806d41a2a88594f562b110e9" +CATEGORY_PROJECT = "Category:OSWb2d7e6a2eff94c82b7f1f2699d5b0ee3" + +# Fixed vocabularies. Both were verified on arkeve: each category holds +# exactly these three items and no others. The ids are the items' uuids and +# come from the shared OSL core data model, so they are the same on every +# instance that imports it; spot-checked on two further instances. An +# instance that defines its own items is still reachable, because +# _resolve_vocab passes a value already starting with "Item:" through. +STATUS_ITEMS = { + "to do": "Item:OSWaa8d29404288446a9f3ec7afa4e2a512", + "in work": "Item:OSWa2b4567ad4874ea1b9adfed19a3d06d1", + "done": "Item:OSWf474ec34b7df451ea8356134241aef8a", +} +PRIO_ITEMS = { + "high": "Item:OSW8743c7d03c4e46c1bd42bb05e1a082d9", + "medium": "Item:OSW8d781c35212548fa9b2fccad3765da65", + "low": "Item:OSWcaf7db070ad6407babc5245e84d76840", +} +# A few aliases so an agent mapping free text does not fail on wording. No +# aliases for priority; high, medium and low are unambiguous. +STATUS_ALIASES = { + "todo": "to do", + "open": "to do", + "pending": "to do", + "backlog": "to do", + "in progress": "in work", + "doing": "in work", + "wip": "in work", + "closed": "done", + "complete": "done", + "completed": "done", + "finished": "done", +} + +# SMW property names, read from the @context of the Process and Task schemas +# on arkeve. The JSON field name and the SMW property name differ, so +# queries must use the property name. +PROP_STATUS = "HasStatus" +PROP_PRIO = "HasPriority" +PROP_RELATED_TO = "IsRelatedTo" +PROP_ACTIONEE = "HasActionee" +PROP_LABEL = "HasLabel" + +_DATE_RE = re.compile(r"^\d{4}-\d{2}-\d{2}$") + + +def _write_category(kind: str) -> str: + """Return the category a newly created 'task', 'person' or 'project' gets. + + A configured override wins; otherwise the core constant. Reading always + uses the core constant instead, since MediaWiki category membership + includes the whole subclass tree. + """ + settings = config.get_settings() + overrides = { + "task": settings.task_category, + "person": settings.person_category, + "project": settings.project_category, + } + defaults = { + "task": CATEGORY_TASK, + "person": CATEGORY_PERSON, + "project": CATEGORY_PROJECT, + } + return overrides[kind] or defaults[kind] + + +def _ensure_models(ctx: Context) -> None: + """Make sure the generated Task/Person/Project model classes exist. + + Fetches the schemas only when a class is missing, guarded on + ``_resolve_category_class(...) is not None`` rather than on a guessed + class name, since ``fetch_schema`` rewrites the installed + ``osw.model.entity`` module and reloads it, which is why this must run + at most once per process. + """ + wanted = [ + CATEGORY_TASK, + _write_category("task"), + CATEGORY_PERSON, + _write_category("person"), + CATEGORY_PROJECT, + _write_category("project"), + ] + missing = [c for c in dict.fromkeys(wanted) if _resolve_category_class(c) is None] + if not missing: + return + # fetch_schema turns the site page cache on and does not always turn it + # back off. A cache left on makes a later read return a page revision + # from before a write in the same process, so restore the state here. + cache_state = ctx.osw.site.get_cache_enabled() + try: + fetch = ctx.osw.fetch_schema( + OSW.FetchSchemaParam(schema_title=missing, mode="append") + ) + finally: + if cache_state: + ctx.osw.site.enable_cache() + else: + ctx.osw.site.disable_cache() + if fetch.error_messages: + raise errors.SchemaError("; ".join(fetch.error_messages)) + + +def _get_page_uncached(ctx: Context, title: str): + """Download a page, bypassing the site page cache. + + An update must read the revision that is on the server right now. A + cached page object can hold a revision from before a write made earlier + in the same process, which would make the update write old field values + back. + """ + cache_state = ctx.osw.site.get_cache_enabled() + ctx.osw.site.disable_cache() + try: + return ctx.osw.site.get_page(WtSite.GetPageParam(titles=[title])).pages[0] + finally: + if cache_state: + ctx.osw.site.enable_cache() + + +def _ask_rows( + ctx: Context, query: str, printouts: list[str], limit: Optional[int] = None +) -> tuple[list[dict], bool]: + """Run an SMW ask query that returns property values, not only titles.""" + lim = ctx.limit(limit) + full = query + "".join(f"|?{p}" for p in printouts) + raw = ctx.osw.site.semantic_search( + WtSite.SearchParam(query=full, limit=lim, return_json=True) + ) + response = raw[0] if raw else {} + payload = response.get("query", {}).get("results", {}) + if not isinstance(payload, dict): + # SMW serialises an empty result set as a JSON array, not an object. + payload = {} + rows = [p for p in payload.values() if p.get("exists") == "1"] + # SMW marks a cut result set with a top-level continuation offset. A + # complete set of exactly 'limit' rows carries no such key, so comparing + # the row count against the limit would report a truncation that did not + # happen. Measured on arkeve: limit=2 of 408 tasks gives the key, limit=500 + # does not. + return rows, "query-continue-offset" in response + + +def _page_values(row: dict, prop: str) -> list[dict]: + """Return [{"title": ..., "label": ...}, ...] from one page-valued printout.""" + entries = row.get("printouts", {}).get(prop) or [] + return [ + {"title": e["fulltext"], "label": e.get("displaytitle") or e["fulltext"]} + for e in entries + ] + + +def _first_page_value(row: dict, prop: str) -> Optional[dict]: + """The first entry of ``_page_values``, or ``None``.""" + values = _page_values(row, prop) + return values[0] if values else None + + +def _label_of(row: dict) -> str: + """The display label of an ask result row. + + Does not request a ``|?Display_title_of`` printout: its printout key + comes back translated into the wiki's content language. The top-level + ``displaytitle`` is always present and is not translated away. + """ + return row.get("displaytitle") or row["fulltext"] + + +def _resolve_vocab(value: str, table: dict, aliases: dict, field: str) -> str: + """Map a human word to an ``Item:OSW...`` page name. + + A value already starting with ``Item:`` passes through unchanged, which + lets a caller name a value an instance added itself. + """ + if value.startswith("Item:"): + return value + key = value.lower().strip() + key = aliases.get(key, key) + if key not in table: + accepted = ", ".join(sorted(table)) + raise errors.ValidationError( + f"Invalid {field} '{value}'. Accepted values: {accepted}." + ) + return table[key] + + +def _check_injection(value: str, field: str) -> None: + """Reject a value that could change an ask query's structure.""" + if "]]" in value or "[[" in value or "|" in value: + raise errors.ValidationError(f"{field} must not contain ']]', '[[' or '|'.") + + +def _resolve_ref(ctx: Context, value: str, category: str, kind: str) -> str: + """Turn a project or person name (or an existing page name) into a page name.""" + if value.startswith("Item:"): + return value + _check_injection(value, kind) + rows, _ = _ask_rows( + ctx, f"[[{category}]][[{PROP_LABEL}::~*{value}*]]", [], limit=10 + ) + if len(rows) == 1: + return rows[0]["fulltext"] + if not rows: + list_op = "list_projects" if kind == "project" else "list_persons" + raise errors.NotFound( + f"No {kind} matches '{value}'. Use '{list_op}' to see candidates." + ) + candidates = "; ".join(f"{_label_of(r)} ({r['fulltext']})" for r in rows) + raise errors.ValidationError( + f"Multiple {kind}s match '{value}': {candidates}. Pass the page name " + "to choose one." + ) + + +def _resolve_due(value: str) -> str: + """Normalize a due date/time to the ``end_date_time`` field's ISO 8601 form. + + Accepts ``YYYY-MM-DD`` (turned into midnight UTC) or a full ISO 8601 + timestamp; anything else is rejected. + """ + text = value.strip() + date_only = bool(_DATE_RE.match(text)) + # The regex only checks the shape, so parse as well; without this an + # impossible date such as 2026-13-45 would pass straight through. + candidate = f"{text}T00:00:00" if date_only else text.replace("Z", "+00:00") + try: + datetime.fromisoformat(candidate) + except ValueError: + raise errors.ValidationError( + f"Invalid due date/time '{value}'. Use 'YYYY-MM-DD' or full ISO 8601." + ) + return f"{text}T00:00:00Z" if date_only else text + + +def _escape_cell(text: str) -> str: + return text.replace("|", "\\|") + + +def _render_markdown(tasks: list[dict]) -> str: + """Render ``list_tasks``' tasks as a Markdown table. + + The single rendering function for tasks; both ``list_tasks(markdown=True)`` + and ``render_task_view`` call it and nothing else renders tasks. + """ + lines = [ + "| Task | Status | Priority | Project | Actionees |", + "| --- | --- | --- | --- | --- |", + ] + for task in tasks: + task_cell = f"[{_escape_cell(task['label'])}]({task['url']})" + status_cell = _escape_cell(task["status"] or "") + prio_cell = _escape_cell(task["prio"] or "") + project_cell = _escape_cell("; ".join(p["label"] for p in task["related_to"])) + actionees_cell = _escape_cell("; ".join(a["label"] for a in task["actionees"])) + lines.append( + f"| {task_cell} | {status_cell} | {prio_cell} | {project_cell} | " + f"{actionees_cell} |" + ) + return "\n".join(lines) + + +def _resolved_refs(jsondata: dict) -> dict: + """Copy the page-reference lists out of ``jsondata`` before it is stored. + + ``_store`` empties ``related_to`` and ``actionees`` in the dict it is + given: building the pydantic model shares the list objects, and storing + the entity clears them in place. The page itself is written correctly, so + only a caller reading the dict afterwards is affected. Measured on + osl.dev.afin-data.de, 2026-09-17. + """ + return { + "related_to": list(jsondata.get("related_to") or []), + "actionees": list(jsondata.get("actionees") or []), + } + + +def _store(ctx: Context, category: str, jsondata: dict, comment: str) -> dict: + """Build the entity against ``category``'s model and store it. + + ``overwrite=true`` is correct here because both callers (``create_task`` + and ``update_task``) send the complete record, so there is nothing to + preserve on the server. + """ + _ensure_models(ctx) + cls = _resolve_category_class(category) + if cls is None: + raise errors.ClassNotFound( + f"Could not resolve a model class for '{category}' after " + "fetching its schema." + ) + try: + entity = cls(**jsondata) + except Exception as exc: + raise errors.ValidationError( + f"jsondata does not validate against {category}: {exc}" + ) + store = ctx.osw.store_entity( + OSW.StoreEntityParam( + entities=[entity], + overwrite=OverwriteOptions.true, + edit_comment=comment, + bot_edit=True, + ) + ) + titles = list(store.pages.keys()) + # store_entity reports a page title even when the upload was skipped, so + # confirm the pages are really there. Without this an operation can + # return a title and a change_id for a page that was never written. + absent = [t for t in titles if not _get_page_uncached(ctx, t).exists] + if absent: + raise errors.OpError( + f"Storing {category} reported success but these pages do not " + f"exist: {', '.join(absent)}. Do not repeat the write before " + f"opening one of them; on a replicated wiki the read can be " + f"served by a replica that has not caught up yet." + ) + domain = config.get_active_domain() + return { + "titles": titles, + "change_id": store.change_id, + "urls": [f"https://{domain}/wiki/{t}" for t in titles], + } + + +@operation( + group="task", + cli_name="create", + writes=True, + destructive_hint=False, + idempotent_hint=False, + records=lambda r: [ + LedgerRecord( + title=t, op="create_task", change_id=r["change_id"], slots=["jsondata"] + ) + for t in r["titles"] + ], +) +def create_task( + ctx: Context, + label: str, + description: Optional[str] = None, + status: Optional[str] = None, + prio: Optional[str] = None, + project: Optional[str] = None, + actionees: Optional[list[str]] = None, + due: Optional[str] = None, + lang: str = "en", +) -> dict: + """Create a new task. + + ``status`` accepts a human word (to do, in work, done, or an alias like + 'in progress') and defaults to "to do" when omitted. ``prio`` accepts + high, medium or low and is left unset when omitted. ``project`` and each + of ``actionees`` accept either an existing page name (``Item:OSW...``) + or a label to look up; an absent or ambiguous match raises rather than + guessing. When ``actionees`` is omitted entirely, no actionee is + assigned; the operator is never assigned silently. ``due`` maps to the + task's end time, since the Task category has no due-date property; + accepts ``YYYY-MM-DD`` (midnight UTC) or a full ISO 8601 timestamp. + + Use ``list_tasks`` filtered by ``project`` first as a duplicate check: an + exact label match within the same project means the task already exists. + + Returns ``{title, url, uuid, change_id, titles, urls, related_to, + actionees}`` for the created page. ``related_to`` and ``actionees`` are + the page names the given labels resolved to, so the caller can confirm + which entity was chosen; a label search matches a substring, so a single + match is accepted without any further confirmation. + """ + task_uuid = str(uuid4()) + jsondata: dict = { + "type": [_write_category("task")], + "uuid": task_uuid, + "label": [{"text": label, "lang": lang}], + } + if description is not None: + jsondata["description"] = [{"text": description, "lang": lang}] + jsondata["status"] = _resolve_vocab( + status or "to do", STATUS_ITEMS, STATUS_ALIASES, "status" + ) + if prio is not None: + jsondata["prio"] = _resolve_vocab(prio, PRIO_ITEMS, {}, "prio") + if project is not None: + jsondata["related_to"] = [ + _resolve_ref(ctx, project, CATEGORY_PROJECT, "project") + ] + if actionees is not None: + jsondata["actionees"] = [ + _resolve_ref(ctx, a, CATEGORY_PERSON, "person") for a in actionees + ] + if due is not None: + jsondata["end_date_time"] = _resolve_due(due) + + # Copy the resolved references before storing. The page is written + # correctly, but building the model empties these lists inside the dict + # passed to it, so reading them afterwards would report nothing. + resolved = _resolved_refs(jsondata) + result = _store( + ctx, _write_category("task"), jsondata, "Created by osw task create" + ) + return { + "title": result["titles"][0], + "url": result["urls"][0], + "uuid": task_uuid, + "change_id": result["change_id"], + "titles": result["titles"], + "urls": result["urls"], + **resolved, + } + + +@operation( + group="task", + cli_name="update", + writes=True, + destructive_hint=False, + idempotent_hint=True, + records=lambda r: [ + LedgerRecord( + title=r["title"], + op="update_task", + change_id=r["change_id"], + slots=["jsondata"], + ) + ], +) +def update_task( + ctx: Context, + title: str, + label: Optional[str] = None, + description: Optional[str] = None, + status: Optional[str] = None, + prio: Optional[str] = None, + project: Optional[str] = None, + actionees: Optional[list[str]] = None, + due: Optional[str] = None, + lang: str = "en", +) -> dict: + """Update a task by merging the given fields into its stored record. + + Only the parameters actually passed are changed; a parameter left as + ``None`` leaves the stored field untouched. Resolution rules for + ``status``, ``prio``, ``project``, ``actionees`` and ``due`` are the + same as ``create_task``. Setting a field to an empty value is out of + scope here; to clear a field, edit the entity with ``osw entity put``. + + ``project`` and ``actionees`` replace the stored list, they do not add to + it. Passing one actionee removes every other actionee the task had, and + passing one project removes every other related project. To add someone, + read the current actionees with ``list_tasks`` and pass the full list. + + Returns ``{title, url, change_id, changed, related_to, actionees}``, where + ``changed`` is the sorted list of field names that were actually written, + and ``related_to``/``actionees`` are the stored page names after the + update, so the caller can confirm which entity each name resolved to. + """ + page = _get_page_uncached(ctx, title) + if not page.exists: + raise errors.NotFound(f"Task '{title}' does not exist.") + stored = page.get_slot_content("jsondata") + jsondata = dict(stored) + changed = [] + + if label is not None: + jsondata["label"] = [{"text": label, "lang": lang}] + changed.append("label") + if description is not None: + jsondata["description"] = [{"text": description, "lang": lang}] + changed.append("description") + if status is not None: + jsondata["status"] = _resolve_vocab( + status, STATUS_ITEMS, STATUS_ALIASES, "status" + ) + changed.append("status") + if prio is not None: + jsondata["prio"] = _resolve_vocab(prio, PRIO_ITEMS, {}, "prio") + changed.append("prio") + if project is not None: + jsondata["related_to"] = [ + _resolve_ref(ctx, project, CATEGORY_PROJECT, "project") + ] + changed.append("related_to") + if actionees is not None: + jsondata["actionees"] = [ + _resolve_ref(ctx, a, CATEGORY_PERSON, "person") for a in actionees + ] + changed.append("actionees") + if due is not None: + jsondata["end_date_time"] = _resolve_due(due) + changed.append("end_date_time") + + # Keep the existing uuid untouched; keep the existing type unless the + # stored record has none. + if not jsondata.get("type"): + jsondata["type"] = [_write_category("task")] + category = jsondata["type"][0] + + resolved = _resolved_refs(jsondata) + result = _store(ctx, category, jsondata, "Updated by osw task update") + return { + "title": result["titles"][0], + "url": result["urls"][0], + "change_id": result["change_id"], + "changed": sorted(changed), + **resolved, + } + + +@operation( + group="task", + cli_name="list", + read_only_hint=True, + idempotent_hint=True, +) +def list_tasks( + ctx: Context, + project: Optional[str] = None, + actionee: Optional[str] = None, + status: Optional[str] = None, + text: Optional[str] = None, + mine: bool = False, + markdown: bool = False, + limit: Optional[int] = None, +) -> dict: + """List tasks, filtered by project, actionee, status and/or label text. + + This is also the duplicate check before creating a task: filter by + ``project`` and compare each returned ``label`` against the label you + are about to create; an exact label match within one project means the + task already exists. ``project`` and ``actionee`` accept a page name or + a label to look up. ``mine=True`` filters to the configured + ``OSW_PERSON_IRI`` and cannot be combined with ``actionee``. + + Returns ``{tasks, count, truncated}``, plus ``markdown`` when + ``markdown=True``. Each task is + ``{title, url, label, status, prio, related_to, actionees}``, where + ``status``/``prio`` are the human labels or ``None`` and + ``related_to``/``actionees`` are lists of ``{title, label}``. + """ + if mine and actionee is not None: + raise errors.ValidationError("Pass either 'mine' or 'actionee', not both.") + + query = f"[[{CATEGORY_TASK}]]" + person_iri = None + if project is not None: + project_title = _resolve_ref(ctx, project, CATEGORY_PROJECT, "project") + query += f"[[{PROP_RELATED_TO}::{project_title}]]" + if mine: + settings = config.get_settings() + if not settings.person_iri: + raise errors.NotConfigured( + "OSW_PERSON_IRI is not configured; it is required to filter " + "tasks assigned to you." + ) + person_iri = settings.person_iri + query += f"[[{PROP_ACTIONEE}::{person_iri}]]" + elif actionee is not None: + actionee_title = _resolve_ref(ctx, actionee, CATEGORY_PERSON, "person") + query += f"[[{PROP_ACTIONEE}::{actionee_title}]]" + if status is not None: + status_title = _resolve_vocab(status, STATUS_ITEMS, STATUS_ALIASES, "status") + query += f"[[{PROP_STATUS}::{status_title}]]" + if text is not None: + _check_injection(text, "text") + query += f"[[{PROP_LABEL}::~*{text}*]]" + + rows, truncated = _ask_rows( + ctx, query, [PROP_STATUS, PROP_PRIO, PROP_RELATED_TO, PROP_ACTIONEE], limit + ) + # A typo in the configured page name gives the same empty result as having + # no tasks, so tell the two apart. The extra read only happens when the + # result is empty. + if person_iri and not rows and not _get_page_uncached(ctx, person_iri).exists: + raise errors.NotConfigured( + f"OSW_PERSON_IRI is set to '{person_iri}', which is not a page on " + "this wiki, so no task can reference it. Use 'list_persons' to " + "find the right page name." + ) + tasks = [] + for row in rows: + status_value = _first_page_value(row, PROP_STATUS) + prio_value = _first_page_value(row, PROP_PRIO) + tasks.append({ + "title": row["fulltext"], + "url": row["fullurl"], + "label": _label_of(row), + "status": status_value["label"] if status_value else None, + "prio": prio_value["label"] if prio_value else None, + "related_to": _page_values(row, PROP_RELATED_TO), + "actionees": _page_values(row, PROP_ACTIONEE), + }) + result = {"tasks": tasks, "count": len(tasks), "truncated": truncated} + if markdown: + result["markdown"] = _render_markdown(tasks) + return result + + +@operation( + group="task", + cli_name="list-projects", + read_only_hint=True, + idempotent_hint=True, +) +def list_projects( + ctx: Context, text: Optional[str] = None, limit: Optional[int] = None +) -> dict: + """List projects, optionally filtered by label text. + + Use this to find a project's page name for ``create_task``'s + ``project`` parameter when a label match would be ambiguous. + + Returns ``{projects, count, truncated}`` where each entry is + ``{title, url, label}``. + """ + query = f"[[{CATEGORY_PROJECT}]]" + if text is not None: + _check_injection(text, "text") + query += f"[[{PROP_LABEL}::~*{text}*]]" + rows, truncated = _ask_rows(ctx, query, [], limit) + projects = [ + {"title": r["fulltext"], "url": r["fullurl"], "label": _label_of(r)} + for r in rows + ] + return {"projects": projects, "count": len(projects), "truncated": truncated} + + +@operation( + group="task", + cli_name="list-persons", + read_only_hint=True, + idempotent_hint=True, +) +def list_persons( + ctx: Context, text: Optional[str] = None, limit: Optional[int] = None +) -> dict: + """List persons, optionally filtered by label text. + + Finds persons stored in subclasses too, because it queries category + membership rather than an exact type match. Search here before calling + ``create_person``; most instances already hold every person you need. + + Returns ``{persons, count, truncated}`` where each entry is + ``{title, url, label}``. + """ + query = f"[[{CATEGORY_PERSON}]]" + if text is not None: + _check_injection(text, "text") + query += f"[[{PROP_LABEL}::~*{text}*]]" + rows, truncated = _ask_rows(ctx, query, [], limit) + persons = [ + {"title": r["fulltext"], "url": r["fullurl"], "label": _label_of(r)} + for r in rows + ] + return {"persons": persons, "count": len(persons), "truncated": truncated} + + +@operation( + group="task", + cli_name="create-person", + writes=True, + destructive_hint=False, + idempotent_hint=False, + records=lambda r: [ + LedgerRecord( + title=t, op="create_person", change_id=r["change_id"], slots=["jsondata"] + ) + for t in r["titles"] + ], +) +def create_person( + ctx: Context, first_name: str, surname: str, email: Optional[str] = None +) -> dict: + """Create a new person entity. + + This is a fallback, not the normal path: most OSL instances create + persons through their own process or workflow, and an instance usually + already holds every person you need. Search with ``list_persons`` first, + and only create one when the person is genuinely absent. + + The label is built here as " ". The Person schema + derives it from the two name fields through a form-editor template, but + that template only runs in the browser form, so a record written through + the API has to carry its own label. + + Returns ``{title, url, uuid, change_id, titles, urls}``. + """ + person_uuid = str(uuid4()) + jsondata: dict = { + "type": [_write_category("person")], + "uuid": person_uuid, + "first_name": first_name, + "surname": surname, + "label": [{"text": f"{first_name} {surname}", "lang": "en"}], + } + if email is not None: + jsondata["email"] = [email] + + result = _store( + ctx, + _write_category("person"), + jsondata, + "Created by osw task create-person", + ) + return { + "title": result["titles"][0], + "url": result["urls"][0], + "uuid": person_uuid, + "change_id": result["change_id"], + "titles": result["titles"], + "urls": result["urls"], + } + + +@operation( + group="task", + cli_name="render", + surfaces=frozenset({"cli"}), + read_only_hint=True, +) +def render_task_view( + ctx: Context, + output_path: str, + project: Optional[str] = None, + actionee: Optional[str] = None, + status: Optional[str] = None, + mine: bool = False, + limit: Optional[int] = None, +) -> dict: + """Render a Markdown table of tasks and write it to a local file. + + CLI only, since ``output_path`` names a local file and the MCP surface + never exposes a path. Filters are the same as ``list_tasks``. + + Returns ``{output_path, count, bytes_written}``. + """ + result = list_tasks( + ctx, + project=project, + actionee=actionee, + status=status, + mine=mine, + markdown=True, + limit=limit, + ) + content = result["markdown"] + data = content.encode("utf-8") + with open(output_path, "w", encoding="utf-8") as stream: + stream.write(content) + return { + "output_path": output_path, + "count": result["count"], + "bytes_written": len(data), + } diff --git a/src/osw/skills/osl-tasks/SKILL.md b/src/osw/skills/osl-tasks/SKILL.md new file mode 100644 index 00000000..7a2aaec2 --- /dev/null +++ b/src/osw/skills/osl-tasks/SKILL.md @@ -0,0 +1,171 @@ +--- +name: osl-tasks +description: Use when importing local todo or note files into an OpenSemanticLab (OSL) wiki as Task entities, or when listing, filtering and updating tasks that already live in OSL. Covers the seven `osw task` commands and the matching MCP tools - create_task, update_task, list_tasks, list_projects, list_persons, create_person - including the duplicate rule, the link marker written back into the local file, and the fixed status and priority vocabularies. +--- + +# OSL task management + +Read todos from local files, create them as Task entities in an OSL wiki, and +read tasks back. The operations are deliberately narrow: each one takes plain +typed parameters and returns a small flat dict. No JSON Schema is ever handed +to you, so you never have to read one. + +## When to use this skill + +- Importing todos or notes from a local file into OSL. +- Listing or filtering the tasks of a project or a person. +- Updating the status, priority, actionees or due date of a task. +- Writing a local Markdown view of the tasks. + +## Setup + +Four environment variables affect these operations. All are optional. + +| Variable | Meaning | +| --- | --- | +| `OSW_PERSON_IRI` | The page name of the operator's own Person entity, for example `Item:OSW8dca...`. Required only by `list_tasks(mine=True)`. | +| `OSW_TASK_CATEGORY` | The category a newly created task is written to. | +| `OSW_PERSON_CATEGORY` | The category a newly created person is written to. | +| `OSW_PROJECT_CATEGORY` | The category used when resolving a project by name. | + +Find the value for `OSW_PERSON_IRI` with: + +``` +osw task list-persons --text "" +``` + +Reading never uses the three category overrides. Listing and searching always +query the shared OSL core category, and MediaWiki category membership includes +the whole subclass tree. An instance that keeps its data in a local subclass, +for example a local "ISC User" subclass of Person, is therefore found without +any configuration. Set the overrides only when a newly created entity has to +land in that local subclass. + +## The operations + +| CLI | MCP tool | Purpose | +| --- | --- | --- | +| `osw task create` | `create_task` | Create one task. | +| `osw task update` | `update_task` | Merge fields into an existing task. | +| `osw task list` | `list_tasks` | List and filter tasks. | +| `osw task list-projects` | `list_projects` | Find a project page name. | +| `osw task list-persons` | `list_persons` | Find a person page name. | +| `osw task create-person` | `create_person` | Fallback only. See below. | +| `osw task render` | not available | Write a Markdown table to a local file. | + +`render` is CLI only, because it names a local path and the MCP surface never +exposes a path. + +## The import loop + +Process one todo at a time. Never create several tasks and then edit the file +once. + +For each todo line: + +1. **If the line already carries an OSW link, the task exists.** Update it or + skip it. Never create. +2. **If it does not, search for a duplicate** with `list_tasks`, filtered by + the project, and compare the returned `label` against the label you are + about to create. +3. **Create the task.** +4. **Write the link back into the file immediately**, before moving to the + next todo. + +Step 4 has to happen before step 1 of the next todo. A task that exists in OSL +but has no marker in the local file looks like a new todo on the next run, and +gets created a second time. If the file edit fails, stop and report the page +name to the user rather than continuing. + +## The marker format + +A Markdown link appended to the todo line: + +``` +- [ ] Fix the parser ([OSW](https:///wiki/Item:OSW1234...)) +``` + +`create_task` returns both `title` and `url`, so use the returned `url` +verbatim. + +## The duplicate rule + +Prefer creating a duplicate over skipping a real task. A duplicate is visible +and reversible. A skipped todo is silent and loses work. + +- A link already on the line is authoritative. It always means skip or update. +- Without a link, an **exact** label match within the same project means + update, not create. +- Anything weaker than an exact label match is a candidate. Show the + candidates to the user and let them decide. Do not resolve it yourself. + +## The vocabularies + +Status and priority are fixed sets of wiki items. Pass the human word; the +operation maps it to the page name. + +- **Status**: `to do`, `in work`, `done`. Aliases are accepted, for example + `todo`, `open`, `backlog`, `in progress`, `wip`, `closed`, `completed`. + A task created without a status gets `to do`. +- **Priority**: `high`, `medium`, `low`. No aliases. A task created without a + priority has none. + +An invalid value raises an error that lists the accepted values. + +**Due dates map to the task's end time.** The Task category has no due-date +property, so `due` is written to `end_date_time`. Pass `YYYY-MM-DD`, which +becomes midnight UTC, or a full ISO 8601 timestamp. + +## Never guess a person or a project + +`project` and each entry of `actionees` accept either a page name +(`Item:OSW...`) or a label to look up. + +- No match raises an error naming the list operation to run. +- Several matches raises an error listing every candidate with its page name. + +Show that list to the user and ask which one they mean. Do not pick one +yourself. Pass the page name once the user has chosen. + +A label search matches a substring, so exactly one match is accepted without +any further question, even when it is not the entity the user meant. +`create_task` and `update_task` return the resolved page names in +`related_to` and `actionees`. Report those names to the user. + +## Person creation is a fallback + +Most OSL instances create persons and users through their own process or +workflow, and an instance usually already holds every person you need. + +1. Search with `list_persons` first. +2. Only if the person is genuinely absent, ask the user whether to create one. +3. Create it with `create_person` and its `first_name` and `surname`. + +Never create a person without asking. + +## Worked example + +``` +# find the project +osw task list-projects --text "ArkEve" + +# look for an existing task before creating +osw task list --project "Item:OSW3660..." --text "parser" + +# create it +osw task create "Fix the parser" --project "Item:OSW3660..." \ + --status "in work" --prio high --due 2026-12-31 + +# later, mark it done +osw task update Item:OSW1234... --status done + +# a local view of everything assigned to me +osw task render tasks.md --mine +``` + +`update` changes only the fields you pass. A field you leave out keeps its +stored value. To clear a field, edit the entity with `osw entity put`. + +`project` and `actionees` replace the stored list, they do not add to it. +Passing one actionee removes every other actionee the task had. To add a +person, read the current actionees with `list_tasks` and pass the full list. diff --git a/tests/test_cli_ops_skill.py b/tests/test_cli_ops_skill.py new file mode 100644 index 00000000..cc8f951d --- /dev/null +++ b/tests/test_cli_ops_skill.py @@ -0,0 +1,77 @@ +"""Unit tests for osw.cli.ops.install_skill (called directly). + +tests/test_cli.py only exercises osw.cli.ops through the typer command tree +(runner.invoke), so this module calls the operation function directly instead, +matching the style of tests/test_service_ops_tasks.py. +""" + +from __future__ import annotations + +from pathlib import Path +from unittest.mock import MagicMock + +import pytest + +from osw.cli import ops +from osw.service import errors, registry +from osw.service.config import Settings +from osw.service.context import Context, Policy + +SKILL_MD_PATH = ( + Path(ops.__file__).resolve().parent.parent / "skills" / "osl-tasks" / "SKILL.md" +) + + +def _ctx() -> Context: + return Context( + Settings(domain="wiki.example.org", username="u", password="p"), + Policy(), + osw=MagicMock(), + ) + + +def test_default_install_copies_skill_md(tmp_path): + result = ops.install_skill(_ctx(), target_dir=str(tmp_path)) + + installed = tmp_path / "osl-tasks" / "SKILL.md" + assert installed.is_file() + assert installed.read_text(encoding="utf-8") == SKILL_MD_PATH.read_text( + encoding="utf-8" + ) + assert "SKILL.md" in result["files"] + + +def test_unknown_name_raises_not_found(tmp_path): + with pytest.raises(errors.NotFound) as exc_info: + ops.install_skill(_ctx(), name="does-not-exist", target_dir=str(tmp_path)) + + assert "osl-tasks" in str(exc_info.value) + + +def test_second_install_without_force_raises_op_error(tmp_path): + ops.install_skill(_ctx(), target_dir=str(tmp_path)) + + installed = tmp_path / "osl-tasks" / "SKILL.md" + original_text = installed.read_text(encoding="utf-8") + + with pytest.raises(errors.OpError): + ops.install_skill(_ctx(), target_dir=str(tmp_path)) + + assert installed.read_text(encoding="utf-8") == original_text + + +def test_second_install_with_force_overwrites(tmp_path): + ops.install_skill(_ctx(), target_dir=str(tmp_path)) + + installed = tmp_path / "osl-tasks" / "SKILL.md" + installed.write_text("clobbered", encoding="utf-8") + + ops.install_skill(_ctx(), target_dir=str(tmp_path), force=True) + + assert installed.read_text(encoding="utf-8") == SKILL_MD_PATH.read_text( + encoding="utf-8" + ) + + +def test_install_skill_is_registered_on_the_cli_surface_only(): + assert registry.REGISTRY["install_skill"].surfaces == frozenset({"cli"}) diff --git a/tests/test_service_ops_tasks.py b/tests/test_service_ops_tasks.py new file mode 100644 index 00000000..aaafd9f3 --- /dev/null +++ b/tests/test_service_ops_tasks.py @@ -0,0 +1,559 @@ +"""Unit tests for osw.service.ops.tasks (Operation.fn called directly). + +Importing ``osw.service.ops.tasks`` registers its operations in +``osw.service.registry.REGISTRY`` at import time, so this module must not +clear the registry the way ``test_service_registry.py`` does. +""" + +from typing import Any, Optional +from unittest.mock import MagicMock + +import pytest +from opensemantic.base.v1 import OswBaseModel + +from osw.service import errors +from osw.service.config import Settings +from osw.service.context import Context, Policy +from osw.service.ops import tasks + + +def _settings(**overrides) -> Settings: + return Settings(domain="wiki.example.org", username="u", password="p", **overrides) + + +def _osw_with_page(jsondata, exists=True): + page = MagicMock() + page.exists = exists + page.get_slot_content.return_value = jsondata + osw = MagicMock() + osw.site.get_page.return_value.pages = [page] + return osw, page + + +class _FakeEntityModel(OswBaseModel): + """Stand-in for a generated Task/Person model, permissive enough to hold + every field these tests write, so assertions can read them back.""" + + uuid: Optional[Any] = None + type: Optional[Any] = None + label: Optional[Any] = None + description: Optional[Any] = None + status: Optional[Any] = None + prio: Optional[Any] = None + related_to: Optional[Any] = None + actionees: Optional[Any] = None + end_date_time: Optional[Any] = None + first_name: Optional[Any] = None + surname: Optional[Any] = None + email: Optional[Any] = None + + class Config: + extra = "allow" + + +def _ask_row(fulltext, fullurl, displaytitle, printouts=None): + return { + "fulltext": fulltext, + "fullurl": fullurl, + "displaytitle": displaytitle, + "exists": "1", + "printouts": printouts or {}, + } + + +def _ask_response(rows_by_title): + return [{"query": {"results": rows_by_title}}] + + +# -- create_task --------------------------------------------------------- +def test_create_task_minimal_defaults_status_to_do(monkeypatch): + osw = MagicMock() + osw.store_entity.return_value = MagicMock( + pages={"Item:OSW1": MagicMock()}, change_id="c1" + ) + monkeypatch.setattr( + tasks, "_resolve_category_class", lambda category: _FakeEntityModel + ) + monkeypatch.setattr(tasks.config, "get_settings", lambda: _settings()) + monkeypatch.setattr(tasks.config, "get_active_domain", lambda: "wiki.example.org") + ctx = Context(_settings(), Policy(), osw=osw) + + result = tasks.create_task(ctx, label="Do the thing") + + assert result["title"] == "Item:OSW1" + assert result["change_id"] == "c1" + osw.store_entity.assert_called_once() + entity = osw.store_entity.call_args[0][0].entities[0] + assert entity.type == [tasks.CATEGORY_TASK] + assert entity.uuid == result["uuid"] + assert entity.label == [{"text": "Do the thing", "lang": "en"}] + assert entity.status == tasks.STATUS_ITEMS["to do"] + + +def test_create_task_status_alias_maps_to_in_work(monkeypatch): + osw = MagicMock() + osw.store_entity.return_value = MagicMock( + pages={"Item:OSW2": MagicMock()}, change_id="c2" + ) + monkeypatch.setattr( + tasks, "_resolve_category_class", lambda category: _FakeEntityModel + ) + monkeypatch.setattr(tasks.config, "get_settings", lambda: _settings()) + monkeypatch.setattr(tasks.config, "get_active_domain", lambda: "wiki.example.org") + ctx = Context(_settings(), Policy(), osw=osw) + + tasks.create_task(ctx, label="X", status="in progress") + + entity = osw.store_entity.call_args[0][0].entities[0] + assert entity.status == tasks.STATUS_ITEMS["in work"] + + +def test_create_task_unknown_status_raises(monkeypatch): + monkeypatch.setattr(tasks.config, "get_settings", lambda: _settings()) + ctx = Context(_settings(), Policy(), osw=MagicMock()) + + with pytest.raises(errors.ValidationError) as exc_info: + tasks.create_task(ctx, label="X", status="bogus") + + message = str(exc_info.value) + assert "to do" in message + assert "in work" in message + assert "done" in message + + +def test_create_task_writes_project_into_related_to(monkeypatch): + osw = MagicMock() + osw.site.semantic_search.return_value = _ask_response({ + "Item:OSWproj1": _ask_row( + "Item:OSWproj1", + "https://wiki.example.org/wiki/Item:OSWproj1", + "My Project", + ) + }) + osw.store_entity.return_value = MagicMock( + pages={"Item:OSW3": MagicMock()}, change_id="c3" + ) + monkeypatch.setattr( + tasks, "_resolve_category_class", lambda category: _FakeEntityModel + ) + monkeypatch.setattr(tasks.config, "get_settings", lambda: _settings()) + monkeypatch.setattr(tasks.config, "get_active_domain", lambda: "wiki.example.org") + ctx = Context(_settings(), Policy(), osw=osw) + + tasks.create_task(ctx, label="X", project="My Project") + + entity = osw.store_entity.call_args[0][0].entities[0] + assert entity.related_to == ["Item:OSWproj1"] + assert not hasattr(entity, "belongs_to_project") + + +# -- create_person / list_persons ----------------------------------------- +def test_create_person_uses_configured_category_list_persons_uses_core(monkeypatch): + settings = _settings(person_category="Category:OSWoverride") + osw = MagicMock() + osw.store_entity.return_value = MagicMock( + pages={"Item:OSWp1": MagicMock()}, change_id="c4" + ) + monkeypatch.setattr( + tasks, "_resolve_category_class", lambda category: _FakeEntityModel + ) + monkeypatch.setattr(tasks.config, "get_settings", lambda: settings) + monkeypatch.setattr(tasks.config, "get_active_domain", lambda: "wiki.example.org") + ctx = Context(settings, Policy(), osw=osw) + + tasks.create_person(ctx, first_name="Ada", surname="Lovelace") + + entity = osw.store_entity.call_args[0][0].entities[0] + assert entity.type == ["Category:OSWoverride"] + + osw.site.semantic_search.return_value = _ask_response({}) + tasks.list_persons(ctx) + + query = osw.site.semantic_search.call_args[0][0].query + assert query == [f"[[{tasks.CATEGORY_PERSON}]]"] + + +# -- _resolve_ref ---------------------------------------------------------- +def test_resolve_ref_multiple_matches_raises(): + osw = MagicMock() + osw.site.semantic_search.return_value = _ask_response({ + "Item:OSWa": _ask_row( + "Item:OSWa", "https://wiki.example.org/wiki/Item:OSWa", "Alpha" + ), + "Item:OSWb": _ask_row( + "Item:OSWb", "https://wiki.example.org/wiki/Item:OSWb", "Beta" + ), + }) + ctx = Context(_settings(), Policy(), osw=osw) + + with pytest.raises(errors.ValidationError) as exc_info: + tasks._resolve_ref(ctx, "a", tasks.CATEGORY_PROJECT, "project") + + message = str(exc_info.value) + assert "Alpha (Item:OSWa)" in message + assert "Beta (Item:OSWb)" in message + + +def test_resolve_ref_no_matches_raises_not_found(): + osw = MagicMock() + osw.site.semantic_search.return_value = [{"query": {"results": []}}] + ctx = Context(_settings(), Policy(), osw=osw) + + with pytest.raises(errors.NotFound): + tasks._resolve_ref(ctx, "nope", tasks.CATEGORY_PERSON, "person") + + +def test_resolve_ref_rejects_injection_value(): + osw = MagicMock() + ctx = Context(_settings(), Policy(), osw=osw) + + with pytest.raises(errors.ValidationError): + tasks._resolve_ref(ctx, "x]]y", tasks.CATEGORY_PROJECT, "project") + + osw.site.semantic_search.assert_not_called() + + +# -- list_tasks ------------------------------------------------------------- +def test_list_tasks_builds_query_and_parses_fixture_row(): + osw = MagicMock() + project_response = _ask_response({ + "Item:OSWproj1": _ask_row( + "Item:OSWproj1", + "https://wiki.example.org/wiki/Item:OSWproj1", + "My Project", + ) + }) + task_row = { + "fulltext": "Item:OSW227e...", + "fullurl": "https://arkeve.isc.fraunhofer.de/wiki/Item:OSW227e...", + "displaytitle": "(Formular Editor) Abfragen liefern ...", + "exists": "1", + "printouts": { + "HasStatus": [{"fulltext": "Item:OSWa2b4...", "displaytitle": "In work"}], + "HasActionee": [], + }, + } + tasks_response = _ask_response({"Item:OSW227e...": task_row}) + osw.site.semantic_search.side_effect = [project_response, tasks_response] + ctx = Context(_settings(), Policy(), osw=osw) + + result = tasks.list_tasks(ctx, project="My Project", status="in work") + + assert result["count"] == 1 + assert result["truncated"] is False + task = result["tasks"][0] + assert task["title"] == "Item:OSW227e..." + assert task["url"] == "https://arkeve.isc.fraunhofer.de/wiki/Item:OSW227e..." + assert task["label"] == "(Formular Editor) Abfragen liefern ..." + assert task["status"] == "In work" + assert task["prio"] is None + assert task["actionees"] == [] + + second_call_query = osw.site.semantic_search.call_args_list[1][0][0].query[0] + assert second_call_query == ( + f"[[{tasks.CATEGORY_TASK}]]" + f"[[{tasks.PROP_RELATED_TO}::Item:OSWproj1]]" + f"[[{tasks.PROP_STATUS}::{tasks.STATUS_ITEMS['in work']}]]" + f"|?{tasks.PROP_STATUS}|?{tasks.PROP_PRIO}" + f"|?{tasks.PROP_RELATED_TO}|?{tasks.PROP_ACTIONEE}" + ) + + +def test_list_tasks_handles_empty_result_shape(): + osw = MagicMock() + osw.site.semantic_search.return_value = [{"query": {"results": []}}] + ctx = Context(_settings(), Policy(), osw=osw) + + result = tasks.list_tasks(ctx) + + assert result == {"tasks": [], "count": 0, "truncated": False} + + +def test_list_tasks_mine_without_person_iri_raises_not_configured(monkeypatch): + monkeypatch.setattr(tasks.config, "get_settings", lambda: _settings()) + ctx = Context(_settings(), Policy(), osw=MagicMock()) + + with pytest.raises(errors.NotConfigured): + tasks.list_tasks(ctx, mine=True) + + +# -- update_task ------------------------------------------------------------- +def test_update_task_preserves_unnamed_field_and_reports_changed(monkeypatch): + stored = { + "uuid": "u1", + "type": ["Category:OSWtaskcore"], + "label": [{"text": "Old", "lang": "en"}], + "foo": "bar", + } + osw, _ = _osw_with_page(stored) + osw.store_entity.return_value = MagicMock( + pages={"Item:OSWtask1": MagicMock()}, change_id="c5" + ) + monkeypatch.setattr( + tasks, "_resolve_category_class", lambda category: _FakeEntityModel + ) + monkeypatch.setattr(tasks.config, "get_settings", lambda: _settings()) + monkeypatch.setattr(tasks.config, "get_active_domain", lambda: "wiki.example.org") + ctx = Context(_settings(), Policy(), osw=osw) + + result = tasks.update_task(ctx, title="Item:OSWtask1", label="New") + + assert result["changed"] == ["label"] + entity = osw.store_entity.call_args[0][0].entities[0] + assert entity.foo == "bar" + assert entity.uuid == "u1" + assert entity.label == [{"text": "New", "lang": "en"}] + + +def test_update_task_missing_page_raises_not_found(): + osw, _ = _osw_with_page({}, exists=False) + ctx = Context(_settings(), Policy(), osw=osw) + + with pytest.raises(errors.NotFound): + tasks.update_task(ctx, title="Item:OSWmissing", label="New") + + +# -- _ensure_models ------------------------------------------------------------ +def test_ensure_models_fetches_only_when_missing(monkeypatch): + osw = MagicMock() + osw.fetch_schema.return_value = MagicMock(error_messages=[]) + monkeypatch.setattr(tasks.config, "get_settings", lambda: _settings()) + ctx = Context(_settings(), Policy(), osw=osw) + + monkeypatch.setattr(tasks, "_resolve_category_class", lambda category: None) + tasks._ensure_models(ctx) + assert osw.fetch_schema.call_count == 1 + + osw.fetch_schema.reset_mock() + monkeypatch.setattr( + tasks, "_resolve_category_class", lambda category: _FakeEntityModel + ) + tasks._ensure_models(ctx) + assert osw.fetch_schema.call_count == 0 + + +def test_ensure_models_restores_a_disabled_page_cache(monkeypatch): + """fetch_schema turns the page cache on and leaves it on. + + A cache left on makes a later read return a revision from before a write + in the same process, so _ensure_models has to restore the prior state. + """ + osw = MagicMock() + osw.fetch_schema.return_value = MagicMock(error_messages=[]) + osw.site.get_cache_enabled.return_value = False + monkeypatch.setattr(tasks.config, "get_settings", lambda: _settings()) + monkeypatch.setattr(tasks, "_resolve_category_class", lambda category: None) + ctx = Context(_settings(), Policy(), osw=osw) + + tasks._ensure_models(ctx) + + osw.site.disable_cache.assert_called_once() + osw.site.enable_cache.assert_not_called() + + +# -- _store -------------------------------------------------------------------- +def test_store_raises_when_the_written_page_does_not_exist(monkeypatch): + """store_entity reports a title even when it skipped the upload.""" + osw, _ = _osw_with_page({}, exists=False) + osw.store_entity.return_value = MagicMock( + pages={"Item:OSWghost": MagicMock()}, change_id="c6" + ) + monkeypatch.setattr( + tasks, "_resolve_category_class", lambda category: _FakeEntityModel + ) + monkeypatch.setattr(tasks.config, "get_settings", lambda: _settings()) + ctx = Context(_settings(), Policy(), osw=osw) + + with pytest.raises(errors.OpError) as exc_info: + tasks.create_task(ctx, label="X") + + assert "Item:OSWghost" in str(exc_info.value) + + +def test_create_person_sends_a_label_built_from_the_two_name_fields(monkeypatch): + """The schema's label template only runs in the browser form.""" + osw = MagicMock() + osw.site.get_page.return_value.pages = [MagicMock(exists=True)] + osw.store_entity.return_value = MagicMock( + pages={"Item:OSWp2": MagicMock()}, change_id="c7" + ) + monkeypatch.setattr( + tasks, "_resolve_category_class", lambda category: _FakeEntityModel + ) + monkeypatch.setattr(tasks.config, "get_settings", lambda: _settings()) + monkeypatch.setattr(tasks.config, "get_active_domain", lambda: "wiki.example.org") + ctx = Context(_settings(), Policy(), osw=osw) + + tasks.create_person(ctx, first_name="Ada", surname="Lovelace") + + entity = osw.store_entity.call_args[0][0].entities[0] + assert entity.label == [{"text": "Ada Lovelace", "lang": "en"}] + assert entity.first_name == "Ada" + assert entity.surname == "Lovelace" + + +def test_update_task_reads_the_page_with_the_cache_disabled(monkeypatch): + """A cached page object can hold a revision from before an earlier write.""" + osw, _ = _osw_with_page({"uuid": "u2", "type": ["Category:OSWtaskcore"]}) + osw.site.get_cache_enabled.return_value = True + osw.store_entity.return_value = MagicMock( + pages={"Item:OSWtask2": MagicMock()}, change_id="c8" + ) + monkeypatch.setattr( + tasks, "_resolve_category_class", lambda category: _FakeEntityModel + ) + monkeypatch.setattr(tasks.config, "get_settings", lambda: _settings()) + monkeypatch.setattr(tasks.config, "get_active_domain", lambda: "wiki.example.org") + ctx = Context(_settings(), Policy(), osw=osw) + + tasks.update_task(ctx, title="Item:OSWtask2", status="done") + + osw.site.disable_cache.assert_called() + # The caller had the cache on, so it has to be on again afterwards. + assert osw.site.enable_cache.call_count == osw.site.disable_cache.call_count + + +def test_resolve_due_normalizes_a_date_and_rejects_an_impossible_one(): + """The regex only checks the shape, so the calendar date is parsed too.""" + assert tasks._resolve_due("2026-12-31") == "2026-12-31T00:00:00Z" + assert tasks._resolve_due(" 2026-12-31 ") == "2026-12-31T00:00:00Z" + assert tasks._resolve_due("2026-12-31T08:30:00Z") == "2026-12-31T08:30:00Z" + + for bad in ("2026-13-45", "31.12.2026", "next friday", ""): + with pytest.raises(errors.ValidationError): + tasks._resolve_due(bad) + + +def test_render_markdown_escapes_pipes_and_joins_multiple_references(): + rows = [ + { + "label": "Fix the a|b parser", + "url": "https://wiki.example.org/wiki/Item:OSWt1", + "status": "to do", + "prio": None, + "related_to": [{"label": "P1"}, {"label": "P2"}], + "actionees": [{"label": "Ada"}, {"label": "Grace"}], + } + ] + + lines = tasks._render_markdown(rows).splitlines() + + assert lines[0] == "| Task | Status | Priority | Project | Actionees |" + assert lines[1] == "| --- | --- | --- | --- | --- |" + assert lines[2] == ( + r"| [Fix the a\|b parser](https://wiki.example.org/wiki/Item:OSWt1) " + "| to do | | P1; P2 | Ada; Grace |" + ) + + +def test_render_markdown_of_no_tasks_is_a_header_only_table(): + assert tasks._render_markdown([]).splitlines() == [ + "| Task | Status | Priority | Project | Actionees |", + "| --- | --- | --- | --- | --- |", + ] + + +def test_list_tasks_reports_truncation_only_when_smw_sends_a_continue_offset( + monkeypatch, +): + """A complete set of exactly 'limit' rows carries no continuation offset.""" + rows = { + "Item:OSWt1": _ask_row("Item:OSWt1", "https://w/t1", "One"), + "Item:OSWt2": _ask_row("Item:OSWt2", "https://w/t2", "Two"), + } + monkeypatch.setattr(tasks.config, "get_settings", lambda: _settings()) + + osw = MagicMock() + osw.site.semantic_search.return_value = [{"query": {"results": rows}}] + ctx = Context(_settings(), Policy(), osw=osw) + assert tasks.list_tasks(ctx, limit=2)["truncated"] is False + + osw = MagicMock() + osw.site.semantic_search.return_value = [ + {"query": {"results": rows}, "query-continue-offset": 2} + ] + ctx = Context(_settings(), Policy(), osw=osw) + assert tasks.list_tasks(ctx, limit=2)["truncated"] is True + + +def test_list_tasks_mine_raises_when_the_configured_person_page_is_absent(monkeypatch): + """An empty result must not look the same as a typo in OSW_PERSON_IRI.""" + settings = _settings(person_iri="Item:OSWtypo") + monkeypatch.setattr(tasks.config, "get_settings", lambda: settings) + osw, _ = _osw_with_page(None, exists=False) + osw.site.semantic_search.return_value = [{"query": {"results": {}}}] + ctx = Context(settings, Policy(), osw=osw) + + with pytest.raises(errors.NotConfigured) as exc: + tasks.list_tasks(ctx, mine=True) + + assert "Item:OSWtypo" in str(exc.value) + + +def test_list_tasks_mine_does_not_read_the_person_page_when_tasks_are_found( + monkeypatch, +): + settings = _settings(person_iri="Item:OSWme") + monkeypatch.setattr(tasks.config, "get_settings", lambda: settings) + osw, _ = _osw_with_page(None, exists=False) + osw.site.semantic_search.return_value = _ask_response({ + "Item:OSWt1": _ask_row("Item:OSWt1", "https://w/t1", "One") + }) + ctx = Context(settings, Policy(), osw=osw) + + assert tasks.list_tasks(ctx, mine=True)["count"] == 1 + osw.site.get_page.assert_not_called() + + +def test_create_task_returns_the_page_names_its_labels_resolved_to(monkeypatch): + """A label search matches a substring, so the caller must see the choice.""" + osw = MagicMock() + osw.site.semantic_search.side_effect = [ + _ask_response({"Item:OSWproj9": _ask_row("Item:OSWproj9", "u", "ArkEve")}), + _ask_response({"Item:OSWper9": _ask_row("Item:OSWper9", "u", "Ada Lovelace")}), + ] + osw.store_entity.return_value = MagicMock( + pages={"Item:OSWnew": MagicMock()}, change_id="c9" + ) + monkeypatch.setattr( + tasks, "_resolve_category_class", lambda category: _FakeEntityModel + ) + monkeypatch.setattr(tasks.config, "get_settings", lambda: _settings()) + monkeypatch.setattr(tasks.config, "get_active_domain", lambda: "wiki.example.org") + ctx = Context(_settings(), Policy(), osw=osw) + + result = tasks.create_task(ctx, label="Fix it", project="ArkEve", actionees=["Ada"]) + + assert result["related_to"] == ["Item:OSWproj9"] + assert result["actionees"] == ["Item:OSWper9"] + + +def test_create_task_reference_list_survives_the_store_step_clearing_it(monkeypatch): + """Storing empties these lists in the dict it is given; the copy must hold.""" + real_store = tasks._store + + def clearing_store(ctx_, category, jsondata, comment): + result = real_store(ctx_, category, jsondata, comment) + for key in ("related_to", "actionees"): + if key in jsondata: + jsondata[key].clear() + return result + + osw = MagicMock() + osw.site.semantic_search.return_value = _ask_response({ + "Item:OSWper9": _ask_row("Item:OSWper9", "u", "Ada Lovelace") + }) + osw.store_entity.return_value = MagicMock( + pages={"Item:OSWnew2": MagicMock()}, change_id="c10" + ) + monkeypatch.setattr( + tasks, "_resolve_category_class", lambda category: _FakeEntityModel + ) + monkeypatch.setattr(tasks.config, "get_settings", lambda: _settings()) + monkeypatch.setattr(tasks.config, "get_active_domain", lambda: "wiki.example.org") + monkeypatch.setattr(tasks, "_store", clearing_store) + ctx = Context(_settings(), Policy(), osw=osw) + + result = tasks.create_task(ctx, label="Fix it", actionees=["Ada"]) + + assert result["actionees"] == ["Item:OSWper9"] From 0b0f1ba29d34e12a892b9ed5219e79b98e53c580 Mon Sep 17 00:00:00 2001 From: Lukas Gold Date: Fri, 18 Sep 2026 09:38:52 +0200 Subject: [PATCH 02/17] docs(service): correct the cause of the emptied reference lists - oold's LinkedBaseModel.__init__ clears them, not _store - no wiki call is involved; cls(**jsondata) alone is enough - rename the test to match --- src/osw/service/ops/tasks.py | 11 ++++++----- tests/test_service_ops_tasks.py | 8 ++++++-- 2 files changed, 12 insertions(+), 7 deletions(-) diff --git a/src/osw/service/ops/tasks.py b/src/osw/service/ops/tasks.py index d1ebc52e..5f69b342 100644 --- a/src/osw/service/ops/tasks.py +++ b/src/osw/service/ops/tasks.py @@ -288,11 +288,12 @@ def _render_markdown(tasks: list[dict]) -> str: def _resolved_refs(jsondata: dict) -> dict: """Copy the page-reference lists out of ``jsondata`` before it is stored. - ``_store`` empties ``related_to`` and ``actionees`` in the dict it is - given: building the pydantic model shares the list objects, and storing - the entity clears them in place. The page itself is written correctly, so - only a caller reading the dict afterwards is affected. Measured on - osl.dev.afin-data.de, 2026-09-17. + Building the model empties ``related_to`` and ``actionees`` in the dict + it is given. ``cls(**jsondata)`` passes the list objects by reference, and + ``oold.model.v1.LinkedBaseModel.__init__`` moves each page name into + ``__iris__`` by calling ``list.remove`` on that shared object. The entity + and the page it writes are both correct; only a caller reading its own + dict afterwards sees an empty list. Measured with oold 0.16.2. """ return { "related_to": list(jsondata.get("related_to") or []), diff --git a/tests/test_service_ops_tasks.py b/tests/test_service_ops_tasks.py index aaafd9f3..56badd86 100644 --- a/tests/test_service_ops_tasks.py +++ b/tests/test_service_ops_tasks.py @@ -528,8 +528,12 @@ def test_create_task_returns_the_page_names_its_labels_resolved_to(monkeypatch): assert result["actionees"] == ["Item:OSWper9"] -def test_create_task_reference_list_survives_the_store_step_clearing_it(monkeypatch): - """Storing empties these lists in the dict it is given; the copy must hold.""" +def test_create_task_reference_list_survives_being_cleared_by_the_model(monkeypatch): + """oold's LinkedBaseModel empties these lists in the dict it is given. + + The real model is not used here, so the clearing is simulated around + ``_store``; the point is that the returned value is a copy taken before. + """ real_store = tasks._store def clearing_store(ctx_, category, jsondata, comment): From d7943027645dbf4913ab9da955ad9a42370de2a1 Mon Sep 17 00:00:00 2001 From: Lukas Gold Date: Fri, 18 Sep 2026 11:14:46 +0200 Subject: [PATCH 03/17] feat(service): derive SMW property names from the category @context - read the field-to-property mapping from the category schema chain - cache the merged context per WtSite, cleared by clear_cache - keep the hardcoded names as a fallback when the schema is unreadable - alias printouts to the property name; SMW keys them by display label - raise instead of querying a property the @context does not declare --- src/osw/service/ops/tasks.py | 125 +++++++++-- src/osw/wtsite.py | 206 ++++++++++++++++++ tests/test_service_ops_tasks.py | 208 ++++++++++++++++++- tests/test_wtsite_jsonld_context.py | 311 ++++++++++++++++++++++++++++ 4 files changed, 827 insertions(+), 23 deletions(-) create mode 100644 tests/test_wtsite_jsonld_context.py diff --git a/src/osw/service/ops/tasks.py b/src/osw/service/ops/tasks.py index 5f69b342..8f230adc 100644 --- a/src/osw/service/ops/tasks.py +++ b/src/osw/service/ops/tasks.py @@ -9,6 +9,7 @@ from __future__ import annotations import re +import warnings from datetime import datetime from typing import Optional from uuid import uuid4 @@ -57,15 +58,25 @@ "finished": "done", } -# SMW property names, read from the @context of the Process and Task schemas -# on arkeve. The JSON field name and the SMW property name differ, so -# queries must use the property name. +# These are fallbacks used only when the category @context cannot be read; +# the property names are normally derived from the wiki with +# WtSite.get_smw_property_map, so an instance that remaps a property is +# followed automatically. PROP_STATUS = "HasStatus" PROP_PRIO = "HasPriority" PROP_RELATED_TO = "IsRelatedTo" PROP_ACTIONEE = "HasActionee" PROP_LABEL = "HasLabel" +# Maps a JSON field name to its fallback SMW property constant. +_PROP_FALLBACK = { + "status": PROP_STATUS, + "prio": PROP_PRIO, + "related_to": PROP_RELATED_TO, + "actionees": PROP_ACTIONEE, + "label": PROP_LABEL, +} + _DATE_RE = re.compile(r"^\d{4}-\d{2}-\d{2}$") @@ -144,12 +155,72 @@ def _get_page_uncached(ctx: Context, title: str): ctx.osw.site.enable_cache() +def _fallback_props(category: str, fields: list[str]) -> dict: + """Return the fallback SMW property names for ``fields``. + + Raises ``errors.OpError`` instead of silently guessing when a field has + no entry in ``_PROP_FALLBACK``, since a field name is not a valid + property name and would build a query that returns nothing and reports + no error. + """ + missing = [f for f in fields if f not in _PROP_FALLBACK] + if missing: + raise errors.OpError( + f"No built-in fallback Semantic MediaWiki property name for " + f"field(s) {', '.join(missing)} of '{category}'; this operation " + "cannot continue without reading the wiki's @context." + ) + return {f: _PROP_FALLBACK[f] for f in fields} + + +def _smw_props(ctx: Context, category: str, fields: list[str]) -> dict: + """Resolve the SMW property name of each field for a category. + + Reads the names from the category's ``@context``, via + ``WtSite.get_smw_property_map``. That chain is resolved and cached in + ``WtSite``, so it costs one read per category per process, not per call. + If the schema pages cannot be read but the query API can, the fallback in + ``_PROP_FALLBACK`` keeps a read working. + """ + try: + mapping = ctx.osw.site.get_smw_property_map(category) + except Exception as exc: + warnings.warn( + f"Could not read the SMW property map for '{category}': {exc}. " + "Using the built-in property names instead; a query will return " + "nothing if the wiki remapped them." + ) + return _fallback_props(category, fields) + if not mapping: + warnings.warn( + f"The @context of '{category}' resolved to no Semantic MediaWiki " + "properties; the category page is probably missing or " + "unreadable. Using the built-in property names instead; a query " + "will return nothing if the wiki remapped them." + ) + return _fallback_props(category, fields) + result = {} + for field in fields: + if field not in mapping: + raise errors.OpError( + f"The @context of '{category}' declares no Semantic MediaWiki " + f"property for field '{field}', so this operation cannot query " + "it. The field may have been renamed in the wiki's data model." + ) + result[field] = mapping[field] + return result + + def _ask_rows( ctx: Context, query: str, printouts: list[str], limit: Optional[int] = None ) -> tuple[list[dict], bool]: """Run an SMW ask query that returns property values, not only titles.""" lim = ctx.limit(limit) - full = query + "".join(f"|?{p}" for p in printouts) + # The '=name' alias forces the printout key in the result to the property + # name; without it, SMW keys the result by the property's display label, + # which need not equal the property name, and the read-back below looks + # the value up by name. + full = query + "".join(f"|?{p}={p}" for p in printouts) raw = ctx.osw.site.semantic_search( WtSite.SearchParam(query=full, limit=lim, return_json=True) ) @@ -221,8 +292,9 @@ def _resolve_ref(ctx: Context, value: str, category: str, kind: str) -> str: if value.startswith("Item:"): return value _check_injection(value, kind) + prop_label = _smw_props(ctx, category, ["label"])["label"] rows, _ = _ask_rows( - ctx, f"[[{category}]][[{PROP_LABEL}::~*{value}*]]", [], limit=10 + ctx, f"[[{category}]][[{prop_label}::~*{value}*]]", [], limit=10 ) if len(rows) == 1: return rows[0]["fulltext"] @@ -567,11 +639,9 @@ def list_tasks( if mine and actionee is not None: raise errors.ValidationError("Pass either 'mine' or 'actionee', not both.") - query = f"[[{CATEGORY_TASK}]]" + # Check the setting first, so a missing configuration does not first pay + # for a schema read. person_iri = None - if project is not None: - project_title = _resolve_ref(ctx, project, CATEGORY_PROJECT, "project") - query += f"[[{PROP_RELATED_TO}::{project_title}]]" if mine: settings = config.get_settings() if not settings.person_iri: @@ -580,19 +650,32 @@ def list_tasks( "tasks assigned to you." ) person_iri = settings.person_iri - query += f"[[{PROP_ACTIONEE}::{person_iri}]]" + + props = _smw_props( + ctx, CATEGORY_TASK, ["status", "prio", "related_to", "actionees", "label"] + ) + + query = f"[[{CATEGORY_TASK}]]" + if project is not None: + project_title = _resolve_ref(ctx, project, CATEGORY_PROJECT, "project") + query += f"[[{props['related_to']}::{project_title}]]" + if person_iri: + query += f"[[{props['actionees']}::{person_iri}]]" elif actionee is not None: actionee_title = _resolve_ref(ctx, actionee, CATEGORY_PERSON, "person") - query += f"[[{PROP_ACTIONEE}::{actionee_title}]]" + query += f"[[{props['actionees']}::{actionee_title}]]" if status is not None: status_title = _resolve_vocab(status, STATUS_ITEMS, STATUS_ALIASES, "status") - query += f"[[{PROP_STATUS}::{status_title}]]" + query += f"[[{props['status']}::{status_title}]]" if text is not None: _check_injection(text, "text") - query += f"[[{PROP_LABEL}::~*{text}*]]" + query += f"[[{props['label']}::~*{text}*]]" rows, truncated = _ask_rows( - ctx, query, [PROP_STATUS, PROP_PRIO, PROP_RELATED_TO, PROP_ACTIONEE], limit + ctx, + query, + [props["status"], props["prio"], props["related_to"], props["actionees"]], + limit, ) # A typo in the configured page name gives the same empty result as having # no tasks, so tell the two apart. The extra read only happens when the @@ -605,16 +688,16 @@ def list_tasks( ) tasks = [] for row in rows: - status_value = _first_page_value(row, PROP_STATUS) - prio_value = _first_page_value(row, PROP_PRIO) + status_value = _first_page_value(row, props["status"]) + prio_value = _first_page_value(row, props["prio"]) tasks.append({ "title": row["fulltext"], "url": row["fullurl"], "label": _label_of(row), "status": status_value["label"] if status_value else None, "prio": prio_value["label"] if prio_value else None, - "related_to": _page_values(row, PROP_RELATED_TO), - "actionees": _page_values(row, PROP_ACTIONEE), + "related_to": _page_values(row, props["related_to"]), + "actionees": _page_values(row, props["actionees"]), }) result = {"tasks": tasks, "count": len(tasks), "truncated": truncated} if markdown: @@ -639,10 +722,11 @@ def list_projects( Returns ``{projects, count, truncated}`` where each entry is ``{title, url, label}``. """ + prop_label = _smw_props(ctx, CATEGORY_PROJECT, ["label"])["label"] query = f"[[{CATEGORY_PROJECT}]]" if text is not None: _check_injection(text, "text") - query += f"[[{PROP_LABEL}::~*{text}*]]" + query += f"[[{prop_label}::~*{text}*]]" rows, truncated = _ask_rows(ctx, query, [], limit) projects = [ {"title": r["fulltext"], "url": r["fullurl"], "label": _label_of(r)} @@ -669,10 +753,11 @@ def list_persons( Returns ``{persons, count, truncated}`` where each entry is ``{title, url, label}``. """ + prop_label = _smw_props(ctx, CATEGORY_PERSON, ["label"])["label"] query = f"[[{CATEGORY_PERSON}]]" if text is not None: _check_injection(text, "text") - query += f"[[{PROP_LABEL}::~*{text}*]]" + query += f"[[{prop_label}::~*{text}*]]" rows, truncated = _ask_rows(ctx, query, [], limit) persons = [ {"title": r["fulltext"], "url": r["fullurl"], "label": _label_of(r)} diff --git a/src/osw/wtsite.py b/src/osw/wtsite.py index 9071e4c8..67ce2e04 100644 --- a/src/osw/wtsite.py +++ b/src/osw/wtsite.py @@ -194,6 +194,12 @@ def __init__(self, config: Union[WtSiteConfig, WtSiteLegacyConfig]): # ALLOWED_FILE_EXTENSIONS_TTL self._allowed_file_extensions = None + # These two caches are always used, unlike the optional page cache above, + # because schema pages change rarely and resolving a category's @context + # chain costs one page read per level + self._jsonld_context_cache = {} + self._jsonld_page_context_cache = {} + def _get_session_lock(self) -> threading.RLock: """Return the session lock, lazily creating it if absent. @@ -524,6 +530,10 @@ def clear_cache(self): """ del self._page_cache self._page_cache = {} + del self._jsonld_context_cache + self._jsonld_context_cache = {} + del self._jsonld_page_context_cache + self._jsonld_page_context_cache = {} class AllowedFileExtensionsResult(OswBaseModel): """The file extensions a wiki accepts and when that list was read""" @@ -1543,6 +1553,202 @@ def loader(url, options=None): return loader + def _merge_jsonld_context( + self, + title: str, + prefer_external_vocabulary: bool, + max_depth: int, + merged: dict, + seen: set, + ) -> None: + """Merge one page's own JSON-LD context mappings into ``merged``, then + recurse into its parents. + + Parameters + ---------- + title: + Full page title (``Category:...`` or ``JsonSchema:...``) to read. + prefer_external_vocabulary: + Passed through to ``JsonLdContextLoaderParams.prefer_external_vocal`` + when resolving the ``*`` convention for this page. + max_depth: + Remaining recursion depth. Recursion stops once this reaches 0, to + guard against a cycle. + merged: + Dictionary that the resolved mappings are merged into, in place. + Parents are merged before the current page, so a child overrides a + parent. + seen: + Titles already visited. Stops the same page being merged twice, + which also breaks a cycle. + + Returns + ------- + None + The result is written into ``merged`` in place. + """ + if max_depth <= 0 or title in seen: + return + seen.add(title) + + cache_key = (title, prefer_external_vocabulary) + if cache_key not in self._jsonld_page_context_cache: + own = {} + parents = [] + try: + page = self.get_page(WtSite.GetPageParam(titles=[title])).pages[0] + schema = None + if page.exists: + if "JsonSchema:" in title: + schema = page.get_slot_content("main") + else: + schema = page.get_slot_content("jsonschema") + except Exception as exc: + # get_page retries 5 times with sleep(5) in between before it + # raises, so a wiki that cannot serve this page would pay that + # delay again on every call; cache the failure as an empty + # result instead. + _logger.warning( + f"Could not read the JSON-LD context of '{title}': {exc}" + ) + schema = None + if isinstance(schema, str): + schema = json.loads(schema) + if isinstance(schema, dict): + params = WtSite.JsonLdContextLoaderParams( + prefer_external_vocal=prefer_external_vocabulary + ) + context = self._replace_jsonld_context_mapping( + deepcopy(schema.get("@context")), params + ) + entries = context if isinstance(context, list) else [context] + for entry in entries: + if isinstance(entry, str): + parents.append(entry) + elif isinstance(entry, dict): + own.update(entry) + # A category usually names its parent in both @context and + # allOf, and following both means a schema that inherits only + # through allOf still contributes its parent's mappings. + # Duplicates are harmless because `seen` stops the second + # visit. + for ref in schema.get("allOf", []): + if isinstance(ref, dict) and ref.get("$ref"): + parents.append(ref["$ref"]) + self._jsonld_page_context_cache[cache_key] = (own, parents) + + own, parents = self._jsonld_page_context_cache[cache_key] + + for ref in parents: + parent_title = ref.split("/wiki/")[-1].split("?")[0] + if parent_title.startswith("Category:") or parent_title.startswith( + "JsonSchema:" + ): + self._merge_jsonld_context( + parent_title, + prefer_external_vocabulary, + max_depth - 1, + merged, + seen, + ) + + merged.update(deepcopy(own)) + + def get_jsonld_context( + self, + category: str, + prefer_external_vocabulary: bool = False, + max_depth: int = 10, + ) -> dict: + """Resolve a category's effective JSON-LD ``@context`` across its + parent chain. + + A category declares only its own mappings in its ``jsonschema`` slot + and points at its parent with a URL entry in ``@context``, so the + effective context of e.g. ``Category:Task`` is spread over Task, + Process, Item and Entity. Levels are merged parent first, so a child + overrides a parent. Each level passes through + ``_replace_jsonld_context_mapping``, which applies the ``*`` + convention, so ``label`` maps to ``skos:prefLabel`` while the wiki + property sits under ``label*``. + + A shared ancestor reached by two different paths is merged once, at + the first position it is reached, which is correct for the linear + chains OSL uses. + + Parameters + ---------- + category: + Full page title of the category, e.g. + ``Category:OSWc5d4829ed2744a219ba027171c75fa1d``. + prefer_external_vocabulary: + Whether to prefer a mapping to an external vocabulary (e.g. + ``skos``, ``schema``) over one in the wiki's own ``Property:`` + namespace. Pass False, the default, to get the wiki's own + ``Property:`` mappings, which is what a Semantic MediaWiki query + needs. + max_depth: + Maximum number of levels to follow up the parent chain. Guards + against a cycle. + + Returns + ------- + dict + The merged ``@context`` mapping. Always a copy of the cached + value, so a caller cannot corrupt the cache by mutating the + result. + """ + cache_key = (category, prefer_external_vocabulary, max_depth) + if cache_key not in self._jsonld_context_cache: + merged = {} + self._merge_jsonld_context( + category, prefer_external_vocabulary, max_depth, merged, set() + ) + self._jsonld_context_cache[cache_key] = merged + return deepcopy(self._jsonld_context_cache[cache_key]) + + def get_smw_property_map( + self, category: str, max_depth: int = 10 + ) -> Dict[str, str]: + """Derive the Semantic MediaWiki property names for a category's + fields. + + The mapping is derived from the merged ``@context`` (see + ``get_jsonld_context``), so it follows what the wiki currently + declares instead of a hardcoded table. Only fields mapped into the + ``Property:`` namespace appear in the result, so prefix declarations + and fields mapped only to an external vocabulary are excluded. The + returned property names carry no ``Property:`` prefix, which is the + form a Semantic MediaWiki ``ask`` query expects. + + The mapping is one-way and cannot be inverted, because several JSON + fields may share one property, for example ``related_to`` and + ``related_to_project`` both map to ``IsRelatedTo``. + + Parameters + ---------- + category: + Full page title of the category. + max_depth: + Maximum number of levels to follow up the parent chain. Passed + through to ``get_jsonld_context``. + + Returns + ------- + Dict[str, str] + Mapping of JSON field name to Semantic MediaWiki property name, + without the ``Property:`` prefix. + """ + context = self.get_jsonld_context( + category, prefer_external_vocabulary=False, max_depth=max_depth + ) + result: Dict[str, str] = {} + for field, entry in context.items(): + iri = entry.get("@id") if isinstance(entry, dict) else entry + if isinstance(iri, str) and iri.startswith("Property:"): + result[field] = iri[len("Property:") :] + return result + class WtPage: """A wrapper class of mwclient.page, mainly to provide multi-slot page handling""" diff --git a/tests/test_service_ops_tasks.py b/tests/test_service_ops_tasks.py index 56badd86..e2998d90 100644 --- a/tests/test_service_ops_tasks.py +++ b/tests/test_service_ops_tasks.py @@ -21,12 +21,26 @@ def _settings(**overrides) -> Settings: return Settings(domain="wiki.example.org", username="u", password="p", **overrides) +def _smw_prop_map() -> dict: + """The identity SMW property map used by tests that do not target + ``_smw_props`` itself, matching the constants the existing fixtures + already assert against.""" + return { + "status": tasks.PROP_STATUS, + "prio": tasks.PROP_PRIO, + "related_to": tasks.PROP_RELATED_TO, + "actionees": tasks.PROP_ACTIONEE, + "label": tasks.PROP_LABEL, + } + + def _osw_with_page(jsondata, exists=True): page = MagicMock() page.exists = exists page.get_slot_content.return_value = jsondata osw = MagicMock() osw.site.get_page.return_value.pages = [page] + osw.site.get_smw_property_map.return_value = _smw_prop_map() return osw, page @@ -123,6 +137,7 @@ def test_create_task_unknown_status_raises(monkeypatch): def test_create_task_writes_project_into_related_to(monkeypatch): osw = MagicMock() + osw.site.get_smw_property_map.return_value = _smw_prop_map() osw.site.semantic_search.return_value = _ask_response({ "Item:OSWproj1": _ask_row( "Item:OSWproj1", @@ -151,6 +166,7 @@ def test_create_task_writes_project_into_related_to(monkeypatch): def test_create_person_uses_configured_category_list_persons_uses_core(monkeypatch): settings = _settings(person_category="Category:OSWoverride") osw = MagicMock() + osw.site.get_smw_property_map.return_value = _smw_prop_map() osw.store_entity.return_value = MagicMock( pages={"Item:OSWp1": MagicMock()}, change_id="c4" ) @@ -176,6 +192,7 @@ def test_create_person_uses_configured_category_list_persons_uses_core(monkeypat # -- _resolve_ref ---------------------------------------------------------- def test_resolve_ref_multiple_matches_raises(): osw = MagicMock() + osw.site.get_smw_property_map.return_value = _smw_prop_map() osw.site.semantic_search.return_value = _ask_response({ "Item:OSWa": _ask_row( "Item:OSWa", "https://wiki.example.org/wiki/Item:OSWa", "Alpha" @@ -196,6 +213,7 @@ def test_resolve_ref_multiple_matches_raises(): def test_resolve_ref_no_matches_raises_not_found(): osw = MagicMock() + osw.site.get_smw_property_map.return_value = _smw_prop_map() osw.site.semantic_search.return_value = [{"query": {"results": []}}] ctx = Context(_settings(), Policy(), osw=osw) @@ -213,9 +231,29 @@ def test_resolve_ref_rejects_injection_value(): osw.site.semantic_search.assert_not_called() +def test_resolve_ref_queries_the_renamed_label_property_not_the_fallback(): + """Rename the label property to prove the lookup query follows the + ``@context`` of the category rather than ``tasks.PROP_LABEL``.""" + osw = MagicMock() + osw.site.get_smw_property_map.return_value = {"label": "WikiLabel"} + osw.site.semantic_search.return_value = _ask_response({ + "Item:OSWa": _ask_row( + "Item:OSWa", "https://wiki.example.org/wiki/Item:OSWa", "Alpha" + ) + }) + ctx = Context(_settings(), Policy(), osw=osw) + + tasks._resolve_ref(ctx, "a", tasks.CATEGORY_PROJECT, "project") + + query = osw.site.semantic_search.call_args[0][0].query[0] + assert "WikiLabel" in query + assert tasks.PROP_LABEL not in query + + # -- list_tasks ------------------------------------------------------------- def test_list_tasks_builds_query_and_parses_fixture_row(): osw = MagicMock() + osw.site.get_smw_property_map.return_value = _smw_prop_map() project_response = _ask_response({ "Item:OSWproj1": _ask_row( "Item:OSWproj1", @@ -254,13 +292,101 @@ def test_list_tasks_builds_query_and_parses_fixture_row(): f"[[{tasks.CATEGORY_TASK}]]" f"[[{tasks.PROP_RELATED_TO}::Item:OSWproj1]]" f"[[{tasks.PROP_STATUS}::{tasks.STATUS_ITEMS['in work']}]]" - f"|?{tasks.PROP_STATUS}|?{tasks.PROP_PRIO}" - f"|?{tasks.PROP_RELATED_TO}|?{tasks.PROP_ACTIONEE}" + f"|?{tasks.PROP_STATUS}={tasks.PROP_STATUS}|?{tasks.PROP_PRIO}={tasks.PROP_PRIO}" + f"|?{tasks.PROP_RELATED_TO}={tasks.PROP_RELATED_TO}" + f"|?{tasks.PROP_ACTIONEE}={tasks.PROP_ACTIONEE}" ) +def test_list_tasks_queries_the_renamed_properties_not_the_fallbacks(): + """The fixtures above map every field to its fallback name, so they pass + either way. Rename the properties to prove the query follows the + ``@context`` of the category rather than the constants.""" + osw = MagicMock() + osw.site.get_smw_property_map.return_value = { + "status": "WikiStatus", + "prio": "WikiPrio", + "related_to": "WikiRelated", + "actionees": "WikiActionee", + "label": "WikiLabel", + } + osw.site.semantic_search.return_value = [{"query": {"results": []}}] + ctx = Context(_settings(), Policy(), osw=osw) + + tasks.list_tasks(ctx, status="in work", text="editor") + + query = osw.site.semantic_search.call_args_list[0][0][0].query[0] + assert query == ( + f"[[{tasks.CATEGORY_TASK}]]" + f"[[WikiStatus::{tasks.STATUS_ITEMS['in work']}]]" + "[[WikiLabel::~*editor*]]" + "|?WikiStatus=WikiStatus|?WikiPrio=WikiPrio" + "|?WikiRelated=WikiRelated|?WikiActionee=WikiActionee" + ) + for fallback in ( + tasks.PROP_STATUS, + tasks.PROP_PRIO, + tasks.PROP_RELATED_TO, + tasks.PROP_ACTIONEE, + tasks.PROP_LABEL, + ): + assert fallback not in query + + +def test_list_tasks_printouts_are_aliased_to_the_property_name(): + """SMW keys a printout by the property's display label, not by the name + written in the query. Measured on osl.dev.afin-data.de: Property:HasLabel + carries the display label 'Label', so an unaliased '|?HasLabel' comes + back keyed 'Label' instead of 'HasLabel'. The '=name' alias forces the + key back to the property name that ``_page_values``/``_first_page_value`` + look up by.""" + osw = MagicMock() + osw.site.get_smw_property_map.return_value = _smw_prop_map() + osw.site.semantic_search.return_value = [{"query": {"results": []}}] + ctx = Context(_settings(), Policy(), osw=osw) + + tasks.list_tasks(ctx) + + query = osw.site.semantic_search.call_args_list[0][0][0].query[0] + assert f"|?{tasks.PROP_STATUS}={tasks.PROP_STATUS}" in query + assert f"|?{tasks.PROP_PRIO}={tasks.PROP_PRIO}" in query + assert f"|?{tasks.PROP_RELATED_TO}={tasks.PROP_RELATED_TO}" in query + assert f"|?{tasks.PROP_ACTIONEE}={tasks.PROP_ACTIONEE}" in query + + +def test_list_projects_queries_the_renamed_property_not_the_fallback(): + """Rename the label property to prove the query follows the ``@context`` + of the category rather than ``tasks.PROP_LABEL``.""" + osw = MagicMock() + osw.site.get_smw_property_map.return_value = {"label": "WikiLabel"} + osw.site.semantic_search.return_value = [{"query": {"results": []}}] + ctx = Context(_settings(), Policy(), osw=osw) + + tasks.list_projects(ctx, text="arkeve") + + query = osw.site.semantic_search.call_args_list[0][0][0].query[0] + assert query == f"[[{tasks.CATEGORY_PROJECT}]][[WikiLabel::~*arkeve*]]" + assert tasks.PROP_LABEL not in query + + +def test_list_persons_queries_the_renamed_property_not_the_fallback(): + """Rename the label property to prove the query follows the ``@context`` + of the category rather than ``tasks.PROP_LABEL``.""" + osw = MagicMock() + osw.site.get_smw_property_map.return_value = {"label": "WikiLabel"} + osw.site.semantic_search.return_value = [{"query": {"results": []}}] + ctx = Context(_settings(), Policy(), osw=osw) + + tasks.list_persons(ctx, text="ada") + + query = osw.site.semantic_search.call_args_list[0][0][0].query[0] + assert query == f"[[{tasks.CATEGORY_PERSON}]][[WikiLabel::~*ada*]]" + assert tasks.PROP_LABEL not in query + + def test_list_tasks_handles_empty_result_shape(): osw = MagicMock() + osw.site.get_smw_property_map.return_value = _smw_prop_map() osw.site.semantic_search.return_value = [{"query": {"results": []}}] ctx = Context(_settings(), Policy(), osw=osw) @@ -271,11 +397,17 @@ def test_list_tasks_handles_empty_result_shape(): def test_list_tasks_mine_without_person_iri_raises_not_configured(monkeypatch): monkeypatch.setattr(tasks.config, "get_settings", lambda: _settings()) - ctx = Context(_settings(), Policy(), osw=MagicMock()) + osw = MagicMock() + osw.site.get_smw_property_map.return_value = _smw_prop_map() + ctx = Context(_settings(), Policy(), osw=osw) with pytest.raises(errors.NotConfigured): tasks.list_tasks(ctx, mine=True) + # The missing setting must be caught before the schema read that + # resolves the SMW property names. + osw.site.get_smw_property_map.assert_not_called() + # -- update_task ------------------------------------------------------------- def test_update_task_preserves_unnamed_field_and_reports_changed(monkeypatch): @@ -464,11 +596,13 @@ def test_list_tasks_reports_truncation_only_when_smw_sends_a_continue_offset( monkeypatch.setattr(tasks.config, "get_settings", lambda: _settings()) osw = MagicMock() + osw.site.get_smw_property_map.return_value = _smw_prop_map() osw.site.semantic_search.return_value = [{"query": {"results": rows}}] ctx = Context(_settings(), Policy(), osw=osw) assert tasks.list_tasks(ctx, limit=2)["truncated"] is False osw = MagicMock() + osw.site.get_smw_property_map.return_value = _smw_prop_map() osw.site.semantic_search.return_value = [ {"query": {"results": rows}, "query-continue-offset": 2} ] @@ -508,6 +642,7 @@ def test_list_tasks_mine_does_not_read_the_person_page_when_tasks_are_found( def test_create_task_returns_the_page_names_its_labels_resolved_to(monkeypatch): """A label search matches a substring, so the caller must see the choice.""" osw = MagicMock() + osw.site.get_smw_property_map.return_value = _smw_prop_map() osw.site.semantic_search.side_effect = [ _ask_response({"Item:OSWproj9": _ask_row("Item:OSWproj9", "u", "ArkEve")}), _ask_response({"Item:OSWper9": _ask_row("Item:OSWper9", "u", "Ada Lovelace")}), @@ -544,6 +679,7 @@ def clearing_store(ctx_, category, jsondata, comment): return result osw = MagicMock() + osw.site.get_smw_property_map.return_value = _smw_prop_map() osw.site.semantic_search.return_value = _ask_response({ "Item:OSWper9": _ask_row("Item:OSWper9", "u", "Ada Lovelace") }) @@ -561,3 +697,69 @@ def clearing_store(ctx_, category, jsondata, comment): result = tasks.create_task(ctx, label="Fix it", actionees=["Ada"]) assert result["actionees"] == ["Item:OSWper9"] + + +# -- _smw_props -------------------------------------------------------------- +def test_smw_props_uses_values_from_get_smw_property_map(): + osw = MagicMock() + osw.site.get_smw_property_map.return_value = { + "status": "HasStatus", + "label": "HasLabel", + } + ctx = Context(_settings(), Policy(), osw=osw) + + result = tasks._smw_props(ctx, tasks.CATEGORY_TASK, ["status", "label"]) + + assert result == {"status": "HasStatus", "label": "HasLabel"} + osw.site.get_smw_property_map.assert_called_once_with(tasks.CATEGORY_TASK) + + +def test_smw_props_missing_field_raises_op_error(): + osw = MagicMock() + osw.site.get_smw_property_map.return_value = {"status": "HasStatus"} + ctx = Context(_settings(), Policy(), osw=osw) + + with pytest.raises(errors.OpError) as exc_info: + tasks._smw_props(ctx, tasks.CATEGORY_TASK, ["status", "label"]) + + message = str(exc_info.value) + assert "label" in message + assert tasks.CATEGORY_TASK in message + + +def test_smw_props_falls_back_and_warns_when_get_smw_property_map_fails(): + osw = MagicMock() + osw.site.get_smw_property_map.side_effect = RuntimeError("boom") + ctx = Context(_settings(), Policy(), osw=osw) + + with pytest.warns(UserWarning, match=tasks.CATEGORY_TASK): + result = tasks._smw_props(ctx, tasks.CATEGORY_TASK, ["status", "label"]) + + assert result == {"status": tasks.PROP_STATUS, "label": tasks.PROP_LABEL} + + +def test_smw_props_falls_back_and_warns_when_get_smw_property_map_is_empty(): + """An empty mapping means the category page is probably missing or + unreadable, not that the data model dropped every field, so this must + fall back rather than raise ``errors.OpError``.""" + osw = MagicMock() + osw.site.get_smw_property_map.return_value = {} + ctx = Context(_settings(), Policy(), osw=osw) + + with pytest.warns(UserWarning, match=tasks.CATEGORY_TASK): + result = tasks._smw_props(ctx, tasks.CATEGORY_TASK, ["status", "label"]) + + assert result == {"status": tasks.PROP_STATUS, "label": tasks.PROP_LABEL} + + +def test_smw_props_raises_op_error_for_a_field_without_a_fallback(): + """A field outside ``_PROP_FALLBACK`` must not be silently guessed; that + would build a query that returns nothing and reports no error.""" + osw = MagicMock() + osw.site.get_smw_property_map.side_effect = RuntimeError("boom") + ctx = Context(_settings(), Policy(), osw=osw) + + with pytest.raises(errors.OpError) as exc_info: + tasks._smw_props(ctx, tasks.CATEGORY_TASK, ["status", "not_a_real_field"]) + + assert "not_a_real_field" in str(exc_info.value) diff --git a/tests/test_wtsite_jsonld_context.py b/tests/test_wtsite_jsonld_context.py new file mode 100644 index 00000000..cdfb1941 --- /dev/null +++ b/tests/test_wtsite_jsonld_context.py @@ -0,0 +1,311 @@ +"""Unit tests for WtSite.get_jsonld_context and WtSite.get_smw_property_map. + +These tests construct a WtSite without going through ``__init__`` (which would +require network / credentials) and monkeypatch ``get_page`` to serve four +canned schemas modelling the real Task -> Process -> Item -> Entity category +chain, so the parent-chain resolution and its caches can be exercised without +a live wiki. +""" + +import logging +import types + +from osw.wtsite import WtSite + +CATEGORY_TASK = "Category:Task" +CATEGORY_PROCESS = "Category:Process" +CATEGORY_ITEM = "Category:Item" +CATEGORY_ENTITY = "Category:Entity" + + +def _ref(title: str) -> str: + """A parent reference URL, of the shape used in a real @context entry.""" + return f"/wiki/{title}?action=raw&slot=jsonschema" + + +# Entity contributes the root mappings: label (plain -> skos, starred -> +# Property:HasLabel, per the * convention), an external-only field with +# no starred variant, and a prefix declaration. +ENTITY_SCHEMA = { + "@context": [ + { + "label": {"@id": "skos:prefLabel"}, + "label*": {"@id": "Property:HasLabel"}, + "description": {"@id": "schema:description"}, + "Property": {"@id": "wiki:Property-3A", "@prefix": True}, + } + ] +} + +# Item contributes its own field, plus a "status" mapping that its child +# Process overrides. +ITEM_SCHEMA = { + "@context": [ + _ref(CATEGORY_ENTITY), + { + "icon": {"@id": "Property:HasIcon"}, + "status": {"@id": "Property:ItemStatus"}, + }, + ] +} + +# Process contributes status (overriding Item's) and actionees. +PROCESS_SCHEMA = { + "@context": [ + _ref(CATEGORY_ITEM), + { + "status": {"@id": "Property:HasStatus"}, + "actionees": {"@id": "Property:HasActionee"}, + }, + ] +} + +# Task contributes prio and related_to. +TASK_SCHEMA = { + "@context": [ + _ref(CATEGORY_PROCESS), + { + "prio": {"@id": "Property:HasPriority"}, + "related_to": {"@id": "Property:IsRelatedTo"}, + }, + ] +} + +DEFAULT_PAGES = { + CATEGORY_TASK: TASK_SCHEMA, + CATEGORY_PROCESS: PROCESS_SCHEMA, + CATEGORY_ITEM: ITEM_SCHEMA, + CATEGORY_ENTITY: ENTITY_SCHEMA, +} + + +class _FakePage: + def __init__(self, exists: bool, jsonschema, expected_slot: str = "jsonschema"): + self.exists = exists + self._jsonschema = jsonschema + self._expected_slot = expected_slot + + def get_slot_content(self, slot_key, clone: bool = True): + # A ``JsonSchema:`` page keeps its schema in the ``main`` slot, not + # ``jsonschema``; fail loudly if the wrong slot is requested instead + # of silently returning no mappings. + assert slot_key == self._expected_slot + return self._jsonschema + + +def _make_site(pages: dict): + """A minimal WtSite, bypassing __init__, whose get_page is monkeypatched + to serve ``pages`` (title -> schema dict). Returns the site and the list + of titles requested, in request order, so a test can assert on the read + count. + """ + site = WtSite.__new__(WtSite) + site._jsonld_context_cache = {} + site._jsonld_page_context_cache = {} + site._site = types.SimpleNamespace(host="wiki.example.org") + + requested_titles = [] + + def fake_get_page(param): + title = param.titles[0] + requested_titles.append(title) + schema = pages.get(title) + expected_slot = "main" if title.startswith("JsonSchema:") else "jsonschema" + page = _FakePage( + exists=schema is not None, jsonschema=schema, expected_slot=expected_slot + ) + return types.SimpleNamespace(pages=[page]) + + site.get_page = fake_get_page + return site, requested_titles + + +def test_merged_context_contains_mappings_from_every_level(): + site, _ = _make_site(DEFAULT_PAGES) + + context = site.get_jsonld_context(CATEGORY_TASK) + + assert "label" in context # Entity + assert "icon" in context # Item + assert "actionees" in context # Process + assert "prio" in context # Task + + +def test_get_smw_property_map_derives_expected_names(): + site, _ = _make_site(DEFAULT_PAGES) + + props = site.get_smw_property_map(CATEGORY_TASK) + + assert props["status"] == "HasStatus" + assert props["prio"] == "HasPriority" + assert props["related_to"] == "IsRelatedTo" + assert props["actionees"] == "HasActionee" + # The starred key wins over the plain skos:prefLabel mapping. + assert props["label"] == "HasLabel" + + +def test_external_vocabulary_only_field_is_excluded(): + site, _ = _make_site(DEFAULT_PAGES) + + context = site.get_jsonld_context(CATEGORY_TASK) + props = site.get_smw_property_map(CATEGORY_TASK) + + assert context["description"] == {"@id": "schema:description"} + assert "description" not in props + + +def test_prefix_declaration_is_excluded_from_the_property_map(): + site, _ = _make_site(DEFAULT_PAGES) + + props = site.get_smw_property_map(CATEGORY_TASK) + + assert "Property" not in props + + +def test_child_mapping_overrides_parent_mapping(): + site, _ = _make_site(DEFAULT_PAGES) + + props = site.get_smw_property_map(CATEGORY_TASK) + + # Item declares status -> Property:ItemStatus; its child Process + # overrides it with status -> Property:HasStatus. + assert props["status"] == "HasStatus" + + +def test_chain_is_read_once_per_page(): + site, requested_titles = _make_site(DEFAULT_PAGES) + + site.get_jsonld_context(CATEGORY_TASK) + assert sorted(requested_titles) == sorted(DEFAULT_PAGES.keys()) + count_after_first = len(requested_titles) + + # Force the merge to run again by clearing only the top-level cache; the + # per-page cache must still stop a second read of each page. + site._jsonld_context_cache.clear() + site.get_jsonld_context(CATEGORY_TASK) + site._jsonld_context_cache.clear() + site.get_jsonld_context(CATEGORY_TASK) + + assert len(requested_titles) == count_after_first + + +def test_returned_context_is_a_copy(): + site, _ = _make_site(DEFAULT_PAGES) + + first = site.get_jsonld_context(CATEGORY_TASK) + first["mutated"] = "should not leak" + first["label"] = "mutated" + + second = site.get_jsonld_context(CATEGORY_TASK) + + assert "mutated" not in second + assert second["label"] != "mutated" + + +def test_cycle_terminates_and_merges_both_sides(): + cyclic_pages = { + "Category:CycleA": { + "@context": [ + _ref("Category:CycleB"), + {"a_field": {"@id": "Property:HasA"}}, + ] + }, + "Category:CycleB": { + "@context": [ + _ref("Category:CycleA"), + {"b_field": {"@id": "Property:HasB"}}, + ] + }, + } + site, _ = _make_site(cyclic_pages) + + context = site.get_jsonld_context("Category:CycleA") + + assert context["a_field"] == {"@id": "Property:HasA"} + assert context["b_field"] == {"@id": "Property:HasB"} + + +def test_missing_page_contributes_nothing_and_does_not_raise(): + site, _ = _make_site(DEFAULT_PAGES) + + context = site.get_jsonld_context("Category:DoesNotExist") + + assert context == {} + + +def test_jsonschema_parent_is_read_from_the_main_slot(): + """A ``JsonSchema:`` parent keeps its schema in the ``main`` slot, not + ``jsonschema``. ``_make_site``'s fake fails the test outright if the + wrong slot is requested.""" + pages = dict(DEFAULT_PAGES) + pages["Category:WithJsonSchemaParent"] = { + "@context": [ + _ref("JsonSchema:Shared"), + {"tag": {"@id": "Property:HasTag"}}, + ] + } + pages["JsonSchema:Shared"] = { + "@context": [{"shared_field": {"@id": "Property:HasShared"}}] + } + site, _ = _make_site(pages) + + context = site.get_jsonld_context("Category:WithJsonSchemaParent") + + assert context["tag"] == {"@id": "Property:HasTag"} + assert context["shared_field"] == {"@id": "Property:HasShared"} + + +def test_page_read_failure_is_warned_about_and_cached(caplog): + """A page read that raises must not be repeated: get_page retries 5 times + with sleep(5) in between before it raises, so a wiki that cannot serve a + schema page would otherwise pay that delay again on every call.""" + calls = [] + + def flaky_get_page(param): + title = param.titles[0] + calls.append(title) + if title == CATEGORY_ITEM: + raise RuntimeError("network down") + schema = DEFAULT_PAGES.get(title) + page = _FakePage(exists=schema is not None, jsonschema=schema) + return types.SimpleNamespace(pages=[page]) + + site = WtSite.__new__(WtSite) + site._jsonld_context_cache = {} + site._jsonld_page_context_cache = {} + site._site = types.SimpleNamespace(host="wiki.example.org") + site.get_page = flaky_get_page + + with caplog.at_level(logging.WARNING, logger="osw.wtsite"): + context = site.get_jsonld_context(CATEGORY_TASK) + assert CATEGORY_ITEM in caplog.text + + # Item's own mapping, and everything above it (Entity's), is lost. + assert "icon" not in context + assert "label" not in context + # Process and Task, which do not depend on a successful Item read for + # their own mappings, still contribute. + assert "actionees" in context + assert "prio" in context + + calls_after_first_run = len(calls) + # Clear only the top-level cache, like test_chain_is_read_once_per_page, + # so the merge runs again and exercises the per-page cache rather than + # short-circuiting on the already-cached top-level result. + site._jsonld_context_cache.clear() + site.get_jsonld_context(CATEGORY_TASK) + + assert len(calls) == calls_after_first_run + + +def test_max_depth_is_part_of_the_cache_key(): + """A caller that first asks for a truncated depth must not receive the + truncated result when it later asks for the (different) default depth.""" + site, _ = _make_site(DEFAULT_PAGES) + + truncated = site.get_jsonld_context(CATEGORY_TASK, max_depth=2) + full = site.get_jsonld_context(CATEGORY_TASK) + + # Entity, three levels above Task, is out of reach at max_depth=2. + assert "label" not in truncated + assert "label" in full From 219a76e30de623b4eca1c0657c4da7ed2d978b13 Mon Sep 17 00:00:00 2001 From: Lukas Gold Date: Fri, 18 Sep 2026 14:35:18 +0200 Subject: [PATCH 04/17] feat(service): add generic schema, search and validation operations - search ask: printouts parameter, result keys forced by an alias - search label: find an entity or category by its display title - schema get: resolve option merges the category inheritance chain - schema props: expose the Semantic MediaWiki property map - entity validate: check a payload against the resolved schema --- docs/tools/cli.md | 6 +- pyproject.toml | 3 + src/osw/mcp/server.py | 6 +- src/osw/service/context.py | 21 +++ src/osw/service/ops/entities.py | 121 +++++++++++++ src/osw/service/ops/schema.py | 162 ++++++++++++++++- src/osw/service/ops/search.py | 122 +++++++++++-- tests/test_service_ops_entities.py | 225 ++++++++++++++++++++++++ tests/test_service_ops_schema.py | 270 +++++++++++++++++++++++++++++ tests/test_service_ops_search.py | 149 ++++++++++++++++ uv.lock | 2 + 11 files changed, 1066 insertions(+), 21 deletions(-) diff --git a/docs/tools/cli.md b/docs/tools/cli.md index f53f1859..28b83301 100644 --- a/docs/tools/cli.md +++ b/docs/tools/cli.md @@ -32,11 +32,11 @@ Commands are grouped by subject: | Group | Commands | | --- | --- | -| `entity` | `get`, `put`, `export`, `delete` | +| `entity` | `get`, `put`, `export`, `delete`, `validate` | | `file` | `info`, `cat`, `write`, `download`, `upload` | -| `search` | `ask`, `titles`, `content`, `entities`, `sparql` | +| `search` | `ask`, `titles`, `content`, `entities`, `sparql`, `label` | | `slot` | `list`, `get`, `set` | -| `schema` | `get` | +| `schema` | `get`, `props` | | `task` | `create`, `update`, `list`, `list-projects`, `list-persons`, `create-person`, `render` | | `skill` | `install` | | `instances` | `list`, `status` | diff --git a/pyproject.toml b/pyproject.toml index ab07065b..1c8d71f1 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -38,6 +38,9 @@ dependencies = [ "PyLD", "SPARQLWrapper", "jsonpath-ng", + # validates jsondata against a resolved JSON Schema in + # osw.service.ops.entities.validate_entity + "jsonschema>=4.0", "numpy", "pyyaml", "typing_extensions", diff --git a/src/osw/mcp/server.py b/src/osw/mcp/server.py index f8f3812a..c245848a 100644 --- a/src/osw/mcp/server.py +++ b/src/osw/mcp/server.py @@ -35,8 +35,10 @@ Entity and page titles are full MediaWiki page names, e.g. "Item:OSW1234...", never a bare id or label. -Before creating or updating an entity, fetch its category's JSON Schema -(get_category_schema) so the written jsondata validates against it. +Before creating or updating an entity, get its category's JSON Schema with +get_category_schema(resolve=True) - the unresolved schema alone is usually +missing inherited properties - then check the payload with validate_entity +before writing it. This server has no filesystem access: file content moves inline as text, not as a path. For anything path-based (uploading/downloading a local file, the diff --git a/src/osw/service/context.py b/src/osw/service/context.py index 5026891d..152526a4 100644 --- a/src/osw/service/context.py +++ b/src/osw/service/context.py @@ -155,6 +155,27 @@ def page(self, title: str): raise errors.NotFound(f"Page '{title}' does not exist.") return page + def get_page_uncached(self, title: str): + """Return the page for ``title``, bypassing the site page cache. + + The page cache is off by default, but ``OSW.fetch_schema`` turns it + on and only restores the previous state itself when its own call + finishes; a caller that does not save and restore the cache state + around ``fetch_schema`` (e.g. ``create_or_update_entity``) can leave + it on for the rest of a long-running process. A later read through + the plain cached path could then return a page revision from before + a write made earlier in the same process. Mirrors + ``osw.service.ops.tasks._get_page_uncached``. Does not raise for a + missing page; the caller branches on ``page.exists``. + """ + cache_state = self.osw.site.get_cache_enabled() + self.osw.site.disable_cache() + try: + return self.osw.site.get_page(WtSite.GetPageParam(titles=[title])).pages[0] + finally: + if cache_state: + self.osw.site.enable_cache() + def require_write(self, op_name: str) -> None: """Raise if this context's policy disallows writes.""" if not self.policy.allow_writes: diff --git a/src/osw/service/ops/entities.py b/src/osw/service/ops/entities.py index b93c78a7..2a653248 100644 --- a/src/osw/service/ops/entities.py +++ b/src/osw/service/ops/entities.py @@ -2,16 +2,19 @@ from __future__ import annotations +import copy import logging from typing import Annotated, Optional import typer +from jsonschema.validators import validator_for import osw.model.entity as model_entity from osw.core import OSW, AddOverwriteClassOptions, OverwriteOptions from osw.service import config, errors from osw.service.context import Context from osw.service.ledger import LedgerRecord +from osw.service.ops.schema import _resolve_schema from osw.service.params import json_value from osw.service.registry import operation from osw.service.serialization import maybe_truncate, to_jsonable @@ -166,6 +169,124 @@ def create_or_update_entity( } +def _strip_remote_refs(node, path: str, unchecked_refs: list) -> object: + """Return a copy of ``node`` with every remote ``$ref`` replaced by ``{}``. + + A ``$ref`` is remote when its value does not start with ``#`` (a local + JSON pointer); OSL schemas instead point ``$ref`` at a URL that reads + another wiki page's schema, which a JSON Schema validator would try to + fetch over the network. ``{}`` is the empty schema, which accepts + anything, so that part of the payload is simply left unchecked. Each + replacement's location is appended to ``unchecked_refs`` as a JSON path + such as ``$.properties.parent``. + """ + if isinstance(node, dict): + ref = node.get("$ref") + if isinstance(ref, str) and not ref.startswith("#"): + unchecked_refs.append(path) + return {} + return { + key: _strip_remote_refs(value, f"{path}.{key}", unchecked_refs) + for key, value in node.items() + } + if isinstance(node, list): + return [ + _strip_remote_refs(item, f"{path}[{index}]", unchecked_refs) + for index, item in enumerate(node) + ] + return node + + +@operation( + group="entity", cli_name="validate", read_only_hint=True, idempotent_hint=True +) +def validate_entity( + ctx: Context, + category: str, + jsondata: Annotated[dict, typer.Option(parser=json_value)], +) -> dict: + """Check whether ``jsondata`` validates against ``category``'s resolved + JSON Schema, without writing anything. + + Resolves ``category``'s effective JSON Schema across its parent chain - + the same walk ``get_category_schema(resolve=True)`` performs - and + validates ``jsondata`` against it with the ``jsonschema`` package. + Passing here does not guarantee ``create_or_update_entity`` will + succeed: that operation validates by constructing the generated + pydantic model instead, a different check with different rules, and it + also fetches the category's schema and regenerates the local + ``osw.model.entity`` module as a side effect, which this operation never + does. + + ``unknown_fields`` lists top-level keys of ``jsondata`` that are not + declared in the resolved schema's ``properties``. This is the check + that matters most in practice: most OSL schemas do not set + ``additionalProperties: false``, so the validator itself would silently + accept a plausible but wrong field name. ``unchecked_refs`` lists the + JSON paths of parts of the schema that were not checked, because they + were a remote ``$ref`` - a URL pointing at another wiki page rather than + a local JSON pointer; validating against it would require a network + call, so that part of the payload is left unchecked instead. + + Returns ``{category, valid, errors, unknown_fields, unchecked_refs, + sources}``, where ``sources`` are the page titles read while resolving + the schema. An invalid payload is this operation's normal, successful + answer: ``valid`` is False and ``errors`` holds one message per + validation error, each including its JSON path; this never raises for a + bad payload. Raises ``errors.NotFound`` if ``category`` does not exist, + and ``errors.SchemaError`` if the resolved schema could not actually be + read (its ``jsonschema`` slot is missing, empty or unparsable, or + ``category`` is not a category page at all) or is not itself a valid + JSON Schema - in both cases the payload was not checked. + """ + page = ctx.osw.site.get_page(WtSite.GetPageParam(titles=[category])).pages[0] + if not page.exists: + raise errors.NotFound(f"Category '{category}' does not exist.") + + merged_schema, sources = _resolve_schema(ctx, category) + if not merged_schema or not merged_schema.get("properties"): + raise errors.SchemaError( + f"Could not read a JSON Schema for '{category}': its " + "'jsonschema' slot is missing, empty or unparsable, or " + "'category' is not a category page. The payload was not checked." + ) + + # Strip remote $refs on a deep copy, so this validation-only check never + # mutates the schema _resolve_schema built. + unchecked_refs: list = [] + schema = _strip_remote_refs(copy.deepcopy(merged_schema), "$", unchecked_refs) + + try: + validator_cls = validator_for(schema) + validator_cls.check_schema(schema) + validator = validator_cls(schema) + except Exception as exc: + raise errors.SchemaError( + f"The resolved schema for '{category}' is not a valid JSON Schema: {exc}" + ) + + try: + validation_errors = [ + f"{err.json_path}: {err.message}" for err in validator.iter_errors(jsondata) + ] + except Exception as exc: + # An unresolvable local $ref (one iter_errors actually tries to + # follow, unlike the remote ones stripped above) raises instead of + # yielding, so surface it the same way a malformed schema is. + raise errors.SchemaError(f"Could not validate '{category}': {exc}") + declared = set(schema.get("properties") or {}) + unknown_fields = [key for key in jsondata if key not in declared] + + return { + "category": category, + "valid": not validation_errors, + "errors": validation_errors, + "unknown_fields": unknown_fields, + "unchecked_refs": unchecked_refs, + "sources": sources, + } + + @operation( group="entity", cli_name="delete", diff --git a/src/osw/service/ops/schema.py b/src/osw/service/ops/schema.py index 363feb17..3a77b464 100644 --- a/src/osw/service/ops/schema.py +++ b/src/osw/service/ops/schema.py @@ -3,10 +3,104 @@ from __future__ import annotations +import json + +from osw.service import errors from osw.service.context import Context from osw.service.registry import operation from osw.service.serialization import maybe_truncate -from osw.wtsite import WtSite + + +def _merge_schema( + ctx: Context, + title: str, + max_depth: int, + merged: dict, + sources: list[str], + seen: set, +) -> None: + """Merge one page's own JSON Schema into ``merged``, then recurse into + its parents. + + Mirrors ``WtSite._merge_jsonld_context``'s parent-first walk. ``title`` + is a full page title (``Category:...`` or ``JsonSchema:...``); a + ``Category:`` page keeps its schema in the ``jsonschema`` slot, a + ``JsonSchema:`` page in the ``main`` slot. Parent references come from + string entries of the schema's ``@context`` list and from ``$ref`` + values inside ``allOf``. Recursion visits parents before this page + applies its own values, so a child's own value always wins on conflict. + A page that cannot be read, or whose slot does not parse, is skipped + rather than aborting the walk. ``seen`` stops the same page being read + twice within one call, which also breaks a cycle. + """ + if max_depth <= 0 or title in seen: + return + seen.add(title) + + try: + page = ctx.get_page_uncached(title) + schema = None + if page.exists: + if "JsonSchema:" in title: + schema = page.get_slot_content("main") + else: + schema = page.get_slot_content("jsonschema") + if isinstance(schema, str): + schema = json.loads(schema) + except Exception: + schema = None + if not isinstance(schema, dict): + return + sources.append(title) + + parents: list[str] = [] + context = schema.get("@context") + entries = context if isinstance(context, list) else ([context] if context else []) + for entry in entries: + if isinstance(entry, str): + parents.append(entry) + for ref in schema.get("allOf", []) or []: + if isinstance(ref, dict) and ref.get("$ref"): + parents.append(ref["$ref"]) + + for ref in parents: + parent_title = ref.split("/wiki/")[-1].split("?")[0] + if parent_title.startswith("Category:") or parent_title.startswith( + "JsonSchema:" + ): + _merge_schema(ctx, parent_title, max_depth - 1, merged, sources, seen) + + # Apply this page's own schema last, so it overrides its parents: + # properties and definitions merge per key (this page wins on conflict), + # required is a de-duplicated union in first-seen order, and every other + # top-level key is simply taken from this page. + properties = {**merged.get("properties", {}), **schema.get("properties", {})} + required = list( + dict.fromkeys([*merged.get("required", []), *schema.get("required", [])]) + ) + definitions = {**merged.get("definitions", {}), **schema.get("definitions", {})} + merged.update(schema) + merged["properties"] = properties + merged["required"] = required + merged["definitions"] = definitions + + +def _resolve_schema(ctx: Context, category: str, max_depth: int = 10) -> tuple: + """Resolve ``category``'s effective JSON Schema across its parent chain. + + A category's ``jsonschema`` slot only ever declares its own properties + and points at its parent, so this walks the same chain + ``WtSite.get_jsonld_context`` does and merges each level, parent first. + No result is cached: each call may read several pages, which keeps the + result fresh when a category page is edited. + + Returns ``(merged_schema, sources)``, where ``sources`` lists the full + titles of every page successfully read, in the order visited. + """ + merged: dict = {} + sources: list[str] = [] + _merge_schema(ctx, category, max_depth, merged, sources, set()) + return merged, sources @operation( @@ -16,7 +110,7 @@ idempotent_hint=True, max_result_size_chars=200_000, ) -def get_category_schema(ctx: Context, category: str) -> dict: +def get_category_schema(ctx: Context, category: str, resolve: bool = False) -> dict: """Return the JSON Schema of a category (its ``jsonschema`` slot). ``category`` is a full category page name, e.g. ``Category:Item``. The @@ -24,15 +118,73 @@ def get_category_schema(ctx: Context, category: str) -> dict: generating models - does not modify any local files. Use the returned schema to construct a valid ``jsondata`` payload for ``create_or_update_entity``. + + Without ``resolve`` (the default), the schema covers one level only: a + category's own ``jsonschema`` slot, not what it inherits from its + parents. Since an OSL category inherits most of its properties from its + parent chain, the one-level schema is usually not enough on its own to + build a valid payload, even though that is what it is for. Pass + ``resolve=True`` to walk the parent chain and merge it into one schema - + a child's own property always wins over a parent's - which is what + ``create_or_update_entity`` actually needs. The resolved result gains + ``resolved: true`` and a ``sources`` list of every page title read while + resolving it, in the order visited. """ - page = ctx.osw.site.get_page(WtSite.GetPageParam(titles=[category])).pages[0] + page = ctx.get_page_uncached(category) if not page.exists: return {"category": category, "exists": False, "schema": None} - schema = page.get_slot_content("jsonschema") - content, truncated = maybe_truncate(schema, ctx.settings.max_chars) + if not resolve: + schema = page.get_slot_content("jsonschema") + content, truncated = maybe_truncate(schema, ctx.settings.max_chars) + return { + "category": category, + "exists": True, + "schema": content, + "truncated": truncated, + } + merged, sources = _resolve_schema(ctx, category) + content, truncated = maybe_truncate(merged, ctx.settings.max_chars) return { "category": category, "exists": True, "schema": content, "truncated": truncated, + "resolved": True, + "sources": sources, + } + + +@operation( + group="schema", + cli_name="props", + read_only_hint=True, + idempotent_hint=True, +) +def get_category_property_map(ctx: Context, category: str) -> dict: + """Return the Semantic MediaWiki property name for each of a category's fields. + + These are the names to use in an SMW ``ask`` query and in the + ``printouts`` parameter of ``search ask`` - for example, the JSON field + ``status`` typically maps to the property ``HasStatus``. The mapping is + one-way and cannot be inverted: several JSON fields may share one + property. Returned names carry no ``Property:`` prefix. + + Every OSL category chain reaches Entity, which declares at least a + label mapping, so an empty map is never a genuinely property-less + category; it means ``category``'s page is missing or unreadable, or + that ``category`` does not actually name a category. Rather than + returning that as a false-looking empty success, this raises + ``errors.SchemaError`` naming ``category``. + """ + properties = ctx.osw.site.get_smw_property_map(category) + if not properties: + raise errors.SchemaError( + f"'{category}' declares no Semantic MediaWiki properties: its " + "page is missing or unreadable, or 'category' does not name a " + "real category." + ) + return { + "category": category, + "properties": properties, + "count": len(properties), } diff --git a/src/osw/service/ops/search.py b/src/osw/service/ops/search.py index 7942c34f..e14b5556 100644 --- a/src/osw/service/ops/search.py +++ b/src/osw/service/ops/search.py @@ -2,7 +2,7 @@ from __future__ import annotations -from typing import Optional +from typing import List, Optional from osw.core import OSW from osw.service import config, errors @@ -10,7 +10,7 @@ from osw.service.registry import operation from osw.service.serialization import cap_list, to_jsonable from osw.sparql_client_smw import SmwSparqlClient -from osw.wiki_tools import get_query_limit +from osw.wiki_tools import _ask_results_as_dict, get_query_limit from osw.wtsite import WtSite @@ -25,13 +25,25 @@ def _hit_limit(total: int, limit: Optional[int]) -> bool: return bool(limit) and total >= limit +def _check_injection(value: str, field: str) -> None: + """Reject a value that could change an ask query's structure.""" + if "]]" in value or "[[" in value or "|" in value: + raise errors.ValidationError(f"{field} must not contain ']]', '[[' or '|'.") + + @operation( group="search", cli_name="ask", read_only_hint=True, idempotent_hint=True, + max_result_size_chars=200_000, ) -def search_ask(ctx: Context, ask_query: str, limit: Optional[int] = None) -> dict: +def search_ask( + ctx: Context, + ask_query: str, + limit: Optional[int] = None, + printouts: Optional[List[str]] = None, +) -> dict: """Run a Semantic MediaWiki 'ask' query and return matching page titles. This is the only search that can find an entity by a property value, such @@ -56,26 +68,71 @@ def search_ask(ctx: Context, ask_query: str, limit: Optional[int] = None) -> dic ``limit`` defaults to ``OSW_MAX_RESULTS`` (100 when that is unset). A ``limit=N`` written into the query itself wins over it. + + ``printouts`` requests SMW property values alongside each hit, so a + caller does not have to follow up with one ``entity get`` per title. + Give bare property names, such as ``HasStatus``, without the + ``Property:`` prefix; use ``osw schema props`` to discover a category's + property names. Requesting a property the category does not define does + not raise - the row simply carries that key set to ``null``. + ``Display_title_of`` does not work as a printout: its printout key comes + back translated into the wiki's content language regardless of the + alias, so it always resolves to ``null`` here - read an entity's label + from its title or with ``entity get`` instead. When ``printouts`` is + omitted or empty, the query and result are exactly as without it. + Returns ``{titles, count, truncated}``, where ``titles`` are full page names, ``count`` is how many came back once hits whose page does not exist were dropped, and ``truncated`` reports that further matches may - exist beyond them. + exist beyond them. When ``printouts`` is given, the result also carries + ``rows``: a list of ``{title, printouts}`` in the same order as + ``titles``, where ``printouts`` maps each requested name to the raw + value SMW returned for it - a page reference keeps its ``fulltext`` and + ``fullurl`` rather than being flattened to a string. """ lim = ctx.limit(limit) - titles = ctx.osw.site.semantic_search( - WtSite.SearchParam(query=ask_query, limit=lim) - ) # semantic_search lets a 'limit=' written into the query win over `lim`, # so the flag has to compare against the limit that reached the wiki. - # `titles` excludes hits whose page does not exist, so a result set - # thinned that way reads as not truncated. query_limit = get_query_limit(ask_query) effective_limit = lim if query_limit is None else query_limit - capped, total, truncated = cap_list(titles, lim) + + if not printouts: + titles = ctx.osw.site.semantic_search( + WtSite.SearchParam(query=ask_query, limit=lim) + ) + # `titles` excludes hits whose page does not exist, so a result set + # thinned that way reads as not truncated. + capped, total, truncated = cap_list(titles, lim) + return { + "titles": capped, + "count": total, + "truncated": truncated or _hit_limit(total, effective_limit), + } + + # The '=name' alias forces the printout key in the result to the property + # name; without it, SMW keys the result by the property's display label, + # which need not equal the property name, and the row values below are + # looked up by name. + full_query = ask_query + "".join(f"|?{p}={p}" for p in printouts) + raw = ctx.osw.site.semantic_search( + WtSite.SearchParam(query=full_query, limit=lim, return_json=True) + ) + response = raw[0] if raw else {} + payload = _ask_results_as_dict(response.get("query", {}).get("results", {})) + hits = [p for p in payload.values() if p.get("exists") == "1"] + capped_hits, total, truncated = cap_list(hits, lim) + rows = [ + { + "title": hit["fulltext"], + "printouts": {p: hit.get("printouts", {}).get(p) for p in printouts}, + } + for hit in capped_hits + ] return { - "titles": capped, + "titles": [row["title"] for row in rows], "count": total, "truncated": truncated or _hit_limit(total, effective_limit), + "rows": rows, } @@ -220,3 +277,46 @@ def sparql_query( "count": total, "truncated": truncated, } + + +@operation( + group="search", + cli_name="label", + read_only_hint=True, + idempotent_hint=True, +) +def search_by_label( + ctx: Context, + label: str, + category: Optional[str] = None, + limit: Optional[int] = None, +) -> dict: + """Find an entity by its exact display label. + + Runs the ask query ``[[Display_title_of::