Skip to content

Deepen Notion API skill and docs - #27915

Open
Danilodum wants to merge 3 commits into
NousResearch:mainfrom
Danilodum:snaz/notion-api-surface-refresh-20260518
Open

Deepen Notion API skill and docs#27915
Danilodum wants to merge 3 commits into
NousResearch:mainfrom
Danilodum:snaz/notion-api-surface-refresh-20260518

Conversation

@Danilodum

Copy link
Copy Markdown

Summary

  • Refreshes the bundled Notion skill for the current developer surfaces, including REST API 2026-03-11, data sources, pages, blocks, comments, markdown workflows, file uploads, webhooks, MCP, Workers, ntn, and the official JS SDK.
  • Adds a source-backed Notion reference pack covering setup/auth, block/data-source/page workflows, markdown/file-upload/webhook notes, official source mapping, edge-case/codegen notes, and a generated OpenAPI inventory.
  • Adds skills/productivity/notion/scripts/notion_api_surface_snapshot.py for no-credential Notion API/docs/package surface snapshots and drift checks.
  • Updates generated website docs/catalog entries for the Notion bundled skill.
  • Fixes skill_view legacy flat-file lookup so support files under references/, templates/, scripts/, and assets/ do not create false skill-name collisions; adds regression coverage.

Validation

  • git diff --check origin/main...HEAD
  • PYTHONDONTWRITEBYTECODE=1 scripts/run_tests.sh tests/tools/test_skills_tool.py -q -p no:cacheprovider
  • PYTHONDONTWRITEBYTECODE=1 python - <<'PY' ... ast.parse(...) ... PY
  • python3 website/scripts/generate-skill-docs.py plus clean diff check for website/docs/reference/skills-catalog.md and website/docs/user-guide/skills/bundled/productivity/productivity-notion.md

@alt-glitch alt-glitch added type/docs Documentation improvements comp/tools Tool registry, model_tools, toolsets tool/skills Skills system (list, view, manage) P3 Low — cosmetic, nice to have labels May 18, 2026
@teknium1

Copy link
Copy Markdown
Contributor

Thanks for the detailed Notion skill refresh. I verified the skill_view collision premise against current main, but found a few concrete issues to fix before this is safe to salvage.

Problems

  • skills/productivity/notion/SKILL.md:94 and other curl examples use Authorization: Bearer ${NOTI...EY}. ${NOTI...EY} is not a valid shell variable expansion, so copy/pasted commands will fail.
  • skills/productivity/notion/SKILL.md:366 and generated website/docs/user-guide/skills/bundled/productivity/productivity-notion.md:381 have malformed inline markdown: Authorization: Bearer *** and Notion-Version: 2026-03-11is missing the closing backtick after***`.
  • skills/productivity/notion/SKILL.md:14 adds related skills web-apis and oauth-sota, but I found no matching bundled/optional SKILL.md on current main; the generated page leaves them unlinked at website/docs/user-guide/skills/bundled/productivity/productivity-notion.md:24.

Suggested changes

  • Use a real variable such as ${NOTION_API_KEY} or an explicitly non-executable redacted literal in the curl examples, then regenerate the website docs.
  • Fix the checklist backtick typo before regenerating docs.
  • Remove or retarget nonexistent related_skills entries.

Automated hermes-sweeper review.

@teknium1 teknium1 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the substantial Notion API refresh. The underlying Notion skill is still materially stale on current main (skills/productivity/notion/SKILL.md:69 still prescribes 2025-09-03), so the documentation portion is worth salvaging.

Problems

  • The skill_view collision change is already present on current main: tools/skills_tool.py:1172-1180 excludes support paths, and tests/tools/test_skills_tool.py:1228-1261 covers the false-collision case. Omit those redundant hunks during salvage.
  • skills/productivity/notion/SKILL.md:3 is a 187-character description without a final period. AGENTS.md:888-900 requires a <=60-character, one-sentence description ending in a period.
  • skills/productivity/notion/SKILL.md:5 should credit the human contributor first (AGENTS.md:926-931).
  • skills/productivity/notion/SKILL.md:14 names web-apis and oauth-sota, neither of which exists in shipped skill trees; the generated page leaves both unlinked at website/docs/user-guide/skills/bundled/productivity/productivity-notion.md:24.
  • Add the required skill test at tests/skills/test_notion_skill.py (AGENTS.md:948-950).

Suggested changes

  • Keep the Notion refresh, correct metadata/link targets, regenerate docs, and add the skill-level test.
  • The earlier review's malformed authorization-header concern is resolved at this PR head: SKILL.md:94 uses ${NOTION_API_KEY} and SKILL.md:366 has valid inline Markdown.

Automated hermes-sweeper review.

description: "Notion API + ntn CLI: pages, databases, markdown, Workers."
version: 2.0.0
author: community
description: "Use when reading, writing, integrating, or troubleshooting Notion through the REST API, ntn CLI, MCP, webhooks, pages, data sources, markdown, blocks, comments, files, or official JS SDK."

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This description is 187 characters and does not end with a period. AGENTS.md:888-900 requires a single sentence of at most 60 characters ending in a period; please shorten it before regenerating the catalog.

author: community
description: "Use when reading, writing, integrating, or troubleshooting Notion through the REST API, ntn CLI, MCP, webhooks, pages, data sources, markdown, blocks, comments, files, or official JS SDK."
version: 2.1.0
author: community + Hermes Agent

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please credit the human contributor first (for example, Danilo Dumeljic (@Danilodum)), with Hermes Agent only as a secondary collaborator, per AGENTS.md:926-931.

tags: [Notion, Productivity, Notes, Database, API, CLI, Workers]
tags: [Notion, Productivity, Notes, Data Sources, API, CLI, Markdown, Files, Webhooks, MCP]
homepage: https://developers.notion.com
related_skills: [web-apis, oauth-sota, webhook-subscriptions]

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

web-apis and oauth-sota have no matching SKILL.md under either skills/ or optional-skills/, so generated docs render them as dead plain-text references. Remove or retarget them to shipped skills.

Comment thread tools/skills_tool.py
# the thing they document (for example templates/notion.md), and
# counting those as legacy skills creates false collisions with real
# directory skills.
support_dirs = {"references", "templates", "scripts", "assets"}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Current main already excludes support paths in this lookup (tools/skills_tool.py:1172-1180) and has a regression test at tests/tools/test_skills_tool.py:1228-1261. Omit this redundant resolver hunk when salvaging the Notion refresh.

@teknium1 teknium1 added sweeper:risk-compatibility Sweeper risk: may break existing users, config, migrations, defaults, or upgrades sweeper:blast-moderate Sweeper blast radius: moderate — a subsystem or single platform labels Jul 13, 2026

@GottZ GottZ left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This was generated by AI during triage.

Summary

Five PRs are associated with the Notion integration/skill complex. #26 adds an unregistered seven-tool CRUD implementation, #49 only exercises generic tool registration, #52 adds two narrow API helpers, #181 repeats #26's Notion diff alongside an unrelated Windows terminal fix, and #27915 updates the existing bundled skill to the current API and expands its source-backed references.

Related pull requests

  • #26 [closed] related — (+1278/-0) — keep closed: Adds seven Notion API tools, tests, and a skill, but registration is left as a standalone patch snippet rather than integrated into the repository; the contributor review also confirms that a bundled Notion skill already exists. It remains relevant as the source of the Notion code later copied into #181.
  • #49 [closed] related — (+54/-21) — keep closed: Despite its Notion title, the diff adds only a generic hello tool and a WIP README line, provides no Notion operation, and unnecessarily rewrites the tool-discovery list. This does not address stale or incomplete Notion guidance, consistent with the contributor's existing-skill objection.
  • #52 [closed] related — (+29/-0) — keep closed: Adds title search and database-schema retrieval, but these are narrow, unregistered helpers with no error handling or tests, while title search is already covered by the bundled skill's workflow. The contributor review explicitly identifies it as duplicating an existing Notion skill.
  • #181 [closed] related — (+1673/-21) — keep closed and split by concern: Its substantive unique change is the unrelated Windows LocalEnvironment fix; all Notion files are effectively copied from #26 and do not belong in this terminal-fix PR. It remains relevant both as a possible source for a separately scoped Windows fix and as evidence that #26's Notion implementation was duplicated rather than independently advanced.
  • #27915 related — (+3105/-716) — keep open for focused salvage: The diff directly addresses the stale bundled skill by moving guidance from API version 2025-09-03 to 2026-03-11 and adding source-backed coverage for data sources, markdown, uploads, views, webhooks, MCP, CLI, and SDK surfaces. This agrees with the visible keep_open reviews, but their blocking findings must be fixed first: remove already-main skill_view hunks, repair metadata/attribution and nonexistent related skills, add the required skill test, and correct malformed or invalid Authorization examples.

Duplicates

#26 and the Notion portion of #181 are effectively identical; #52 substantially overlaps the existing bundled Notion search/schema guidance rather than providing a distinct integration. #49 is scaffolding, not an implementation duplicate.

Suggested consolidation

Keep #27915 open and salvage only its Notion skill, references, snapshot script, and regenerated documentation; after explicitly resolving both keep_open reviews' blocking findings and rerunning the required skill/docs checks, merge #27915. Keep #26, #49, and #52 closed as superseded or redundant, and keep #181 closed unless its unrelated Windows LocalEnvironment fix is resubmitted as a clean standalone PR without the duplicated #26 Notion files.

Cross-PR triage: Reviewed 5 pull requests and 0 issues in this complex. Each diff was read against this issue; Assessment working set: 300 kB of PR diffs, 5 kB of issue/PR text, 4 kB of discussion (9 comments), 0 verify verdicts. verdicts reflect diff content, not PR titles. Part of an automated triage batch.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

comp/tools Tool registry, model_tools, toolsets P3 Low — cosmetic, nice to have sweeper:blast-moderate Sweeper blast radius: moderate — a subsystem or single platform sweeper:risk-compatibility Sweeper risk: may break existing users, config, migrations, defaults, or upgrades tool/skills Skills system (list, view, manage) type/docs Documentation improvements

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants