Skip to content

fix: use DATA_getSubscription instead of the non-MCP DATA_getCreditBalance - #13

Open
Oleksii Khoroshun (kodotpro) wants to merge 1 commit into
mainfrom
feat/skills-update
Open

fix: use DATA_getSubscription instead of the non-MCP DATA_getCreditBalance#13
Oleksii Khoroshun (kodotpro) wants to merge 1 commit into
mainfrom
feat/skills-update

Conversation

@kodotpro

@kodotpro Oleksii Khoroshun (kodotpro) commented Jun 17, 2026

Copy link
Copy Markdown

Summary

Skills and docs that instruct the agent to call DATA_getCreditBalance are corrected to DATA_getSubscription. Verified against the live SE Ranking MCP: it exposes DATA_getSubscription (0 credits, returns units_left) and not DATA_getCreditBalance, so the old instruction would fail.

Scope (surgical)

Renamed in consuming skills + README + CONTRIBUTING + the centralized preflight reference (14 files).

Deliberately unchanged:

  • seo-api/* — documents credit-balance as a real REST endpoint; byte-identical to main.
  • examples/* — live-evidence logs recorded real REST calls; left intact.

Notes

  • The access gate (hard-stop + quota checks) is split into a separate follow-up PR.
  • Residual DATA_getCreditBalance mentions remain in seo-api MCP-tool columns (SKILL.md:99, rate-limits-and-credits.md:98,187, integration-patterns.md:81) — left for the seo-api author to reconcile, as they touch the documented REST surface.
  • Branch is up to date with main; CI frontmatter linter passes (32/0).

@dominik-stypula-seranking

Copy link
Copy Markdown

Review summary

The core premise is correctDATA_getCreditBalance genuinely is not an MCP tool (verified against seranking/seo-data-api-mcp-server @ main/v2.1.0: zero registration of any credit-balance tool; DATA_getSubscriptionGET /v1/account/subscription is the only account/system tool). Removing it is a real fix worth shipping.

But I'd hold merge until a few things are addressed — most importantly the branch is stale in a way that silently re-breaks the marketplace sync that #16 just repaired.

🔴 Blocker A — merging as-is regresses the Skills-API fix from #16

The PR body says "up to date with main", but the branch is 3 ahead / 12 behind main and predates #16 (description-limit fix) and #17 (the DAT-563 frontmatter linter). Its tree still carries the pre-#16 broken descriptions for the exact 4 files #16 fixed:

skill this branch (bd782c9) main post-#16
seo-api 1388 chars / 1394 bytes (>1024) 998 / 1004 ✅
seo-firecrawl 681, has </> 682, no angle ✅
seo-images 939, has </> 943, no angle ✅
seo-schema 409, has </> 386, no angle ✅

The marketplace sync is atomic, so merging this would reintroduce all four violations and break web/Cowork install again. GitHub's "CLEAN/MERGEABLE" only means no textual conflict — it doesn't catch this. Fix: merge main in (pulling #16 + #17) and keep the compliant descriptions; the new CI linter should then re-validate.

🔴 Blocker B — the rename is incomplete (examples/ untouched)

The PR body claims "…and the seo-api example now uses DATA_getSubscription," but examples/seo-api-seranking-com-20260522/ has zero diff. 8 live references to the phantom tool remain across 4 files, including:

  • code/mcp-calls.md:13,16,18 — instructs callers to run DATA_getCreditBalance and repeats the "~8.6M gap / not interchangeable" note (this file bills itself as "every call verified live")
  • evidence/01-preflight.md:20,27, evidence/02-cost-forecast.md:18, evidence/04-execution-log.md:7,13 — including log rows claiming DATA_getCreditBalance returned HTTP 200

So the example is self-contradictory: it tells users to call — and claims to have live-verified — a tool the rest of the PR declares nonexistent. Fix: update those 4 files, or drop the "every reference" claim.

🟠 Should-fix — the new access gate reads the wrong field path

The gate added to ~21 skills checks units_left, status, units_limit as top-level fields, but DATA_getSubscription returns them wrapped:

{ "subscription_info": { "status": "active", "units_limit": 1500000, "units_left": 0, "start_date": "...", "expiraton_date": "..." } }

…as this PR's own kept reference (seo-api/references/api-surface-map.md) documents. Fix: read subscription_info.*.

🟠 Should-fix — preflight doc contradicts the new gate

seo-firecrawl/references/preflight.md still says credit data is "a courtesy, not a gate" and "a skill never fails the run," while the new per-skill gate says to halt. Fix: reconcile — e.g. tools-absent / quota≤0 = hard stop, but a failed getSubscription call = the soft courtesy path.

🟡 Discuss — semantic correctness of gating on units_left

units_left is subscription plan units, which may diverge from the spendable credit/wallet balance (the Gateway is the source of truth for Data API credits; /v1/account/subscription and /v1/account/credit-balance are different meters). Hard-halting on units_left <= 0, plus deleting the "these don't reconcile" caveat and calling the old ~8.6M gap "not reproducible," may be overconfident — worth verifying the meters still diverge before baking that into the permanent CHANGELOG, and consider warn-and-confirm rather than a hard halt on this number.

🟡 Nits

  • "Free trial" detection (units_limit <= 100000) is an unverified heuristic — no trial flag exists in the response, so small/legacy paid plans get mislabeled.
  • "trial"-word drift (seo-api says "free … trial limits", the other 21 say "free … limits"); curly apostrophes baked into 21 files; the ~80-word gate paragraph is copy-pasted inline into 21 skills despite the centralized preflight doc existing for exactly this.

Suggestion

The tool-rename is sound and should ship — but consider splitting the behavioral access-gate into its own PR (the title is a tool rename; the gate is a cross-cutting behavior change across 21 skills with its own risk profile and the warn-vs-halt decision above).


Multi-reviewer analysis (5 independent passes cross-checking this repo against seranking/seo-data-api-mcp-server). Inspected head bd782c912a12a3242179b40d8bd90fca9d900b71.

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

please check my recent comment in Conversation tab

…lance

Skills and docs that instruct the agent to call DATA_getCreditBalance are
corrected to DATA_getSubscription. Verified against the live SE Ranking MCP:
it exposes DATA_getSubscription (0 credits, returns units_left) and not
DATA_getCreditBalance, so the old instruction would fail.

Scope is surgical: renamed only in consuming skills, README, CONTRIBUTING,
and the centralized preflight reference. seo-api/* (which documents the
credit-balance REST endpoint) and examples/* (live-evidence logs) are left
identical to main.
@kodotpro Oleksii Khoroshun (kodotpro) changed the title fix: use DATA_getSubscription; DATA_getCreditBalance is not an MCP tool fix: use DATA_getSubscription instead of the non-MCP DATA_getCreditBalance Jun 25, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants