Skip to content

feat: add exported API breaking-change detector#1707

Closed
EtoileAI wants to merge 1 commit into
JSONbored:mainfrom
EtoileAI:feat/exported-api-breaking-detector
Closed

feat: add exported API breaking-change detector#1707
EtoileAI wants to merge 1 commit into
JSONbored:mainfrom
EtoileAI:feat/exported-api-breaking-detector

Conversation

@EtoileAI

Copy link
Copy Markdown
Contributor

Summary

  • add a REES exported-API analyzer for issue feat(enrichment): Exported-API breaking-change detector #1510 that compares the currently published npm tarball against the post-PR declaration surface reconstructed from patches
  • detect semver-major exported surface breaks: removed entrypoints, removed exports, and changed exported signatures
  • wire the analyzer into review-enrichment brief generation and rendering, with focused unit/integration coverage for tarball extraction, patch reconstruction, manifest entrypoint overlays, and fail-safe behavior

Scope

  • The PR title follows type(scope): short summary Conventional Commit format, for example fix(api): restore profile access checks.
  • This PR is focused and does not mix unrelated backend, UI, MCP, docs, dependency, and deploy changes.
  • This follows CONTRIBUTING.md and does not reintroduce GitHub Pages, VitePress, site/, or CNAME.
  • I linked an issue, or this is small enough that the summary explains why an issue is not needed.

Linked issue: #1510

Validation

  • git diff --check
  • npm run actionlint
  • npm run typecheck
  • npm run test:coverage locally; codecov/patch requires ≥97% coverage of the lines AND branches you changed (aim for 98%+ on your diff so CI variance does not fail near the threshold). Global coverage is a non-blocking trend with a loose 90% backstop, not the gate.
  • npm run test:workers
  • npm run build:mcp
  • npm run test:mcp-pack
  • npm run ui:openapi:check
  • npm run ui:lint
  • npm run ui:typecheck
  • npm run ui:build
  • npm audit --audit-level=moderate
  • New or changed behavior has unit/integration tests for new branches, fallback paths, and sanitizer boundaries

If any required check was skipped, explain why:

  • none

Safety

  • No secrets, wallet details, hotkeys, coldkeys, user PATs, private keys, raw trust scores, private rankings, or private maintainer evidence are exposed.
  • Public GitHub text stays sanitized, low-noise, and does not imply compensation guarantees or optimization tactics.
  • Auth, cookie, CORS, GitHub App, Cloudflare, or session changes include negative-path tests.
  • API/OpenAPI/MCP behavior is updated and tested where needed.
  • UI changes use live API data or real empty/error/loading states, not production mock/demo fallbacks.
  • Visible UI changes include a UI Evidence section below with JPG/JPEG or PNG screenshots arranged as organized, captioned, clickable thumbnails. SVG screenshots are not used as review evidence. Review-only screenshots or recordings are not committed to the repository.
  • Public docs/changelogs are updated where needed; changelogs are only edited for release-prep PRs.

Notes

  • implementation is intentionally fail-safe: if the package name cannot be derived from the diff, the npm registry/tarball cannot be fetched, or the declaration surface cannot be reconstructed confidently, the analyzer returns no findings instead of guessing
  • the analyzer is scoped to review-enrichment/ and uses the published npm tarball plus PR patches, which aligns with the current REES request shape

@EtoileAI EtoileAI requested a review from JSONbored as a code owner June 29, 2026 04:43
@dosubot dosubot Bot added the size:XL This PR changes 500-999 lines, ignoring generated files. label Jun 29, 2026
@gittensory-orb

gittensory-orb Bot commented Jun 29, 2026

Copy link
Copy Markdown

Warning

🟨🟨🟨🟨🟨🟨🟨🟨🟨🟨🟨🟨

⏸️ Gittensory review — held for maintainer review

5 files · 1 AI reviewer · 1 blocker · readiness 48/100 · CI green · clean

⏸️ Held for maintainer review — This is a first-time contribution to this repo, so the gate stays advisory rather than blocking. The findings remain visible, and the gate will apply normally once this author has merge history here.

Review summary
The PR adds a new exported declaration-surface analyzer, wires it into brief generation, and covers the main tarball/patch/rendering paths. The overall shape is coherent, but the export parser misses a common declaration form and will silently under-report API removals for symbols declared with `export declare type` or `export declare interface`. Rendering and analyzer registration are straightforward.

Blockers

  • review-enrichment/src/analyzers/exported-api.ts:185 parses `export interface` and `export type` but not `export declare interface` or `export declare type`, so a published declaration like `export declare type Options = ...;` is omitted from the baseline surface and its removal will not be reported.

Concerns raised — review before merging

  • AI reviewers agree on a likely critical defect: review-enrichment/src/analyzers/exported-api.ts:185 parses `export interface` and `export type` but not `export declare interface` or `export declare type`, so a published declaration like `export declare type Options = ...;` is omitted from the baseline surface and its removal will not be reported.: review-enrichment/src/analyzers/exported-api.ts:185 parses `export interface` and `export type` but not `export declare interface` or `export declare type`, so a published declaration like `export declare type Options = ...;` is omitted from the baseline surface and its removal will not be reported.
Signal Result Evidence
Code review ❌ 1 blocker 1 reviewer
Linked issue ⚠️ Missing No linked issue or no-issue rationale found.
Related work ⚠️ 3 scoped overlaps Top overlaps are listed below; lower-confidence bulk is hidden.
Review load ❌ 8/20 Readiness component derived from cached public PR metadata and labels; size label size:XL.
Validation evidence ❌ 5/25 Cached preflight status is hold.
Open PR queue ❌ 3/10 15 open PR(s), 9 likely reviewable, 6 unlinked.
Contributor context ✅ Confirmed Gittensor contributor EtoileAI; Gittensor profile; 56 PR(s), 4 issue(s).
Gate result ⚠️ Not blocking Advisory; not blocking this PR.
Nits — 6 non-blocking
  • review-enrichment/src/analyzers/exported-api.ts:627 should catch `JSON.parse(published.packageJson)` failures so one malformed tarball fails closed like the network and extraction paths instead of degrading the whole analyzer run.
  • review-enrichment/src/analyzers/exported-api.ts:465 relies on a line-oriented package.json overlay parser; add coverage for multi-line `exports` objects where unchanged entrypoints are outside the patch context so future edits do not turn partial diffs into false removals.
  • review-enrichment/src/analyzers/exported-api.ts:566 reports a missing head declaration file as `removed-entrypoint` even when the manifest still exposes the entrypoint but points at a missing types path; consider a more precise finding kind or wording to distinguish manifest removal from broken target files.
  • In `review-enrichment/src/analyzers/exported-api.ts:185`, change the interface/type matchers to accept optional `declare`, mirroring the function/class/enum cases, and add a regression test with `export declare type` plus removal.
  • In `review-enrichment/src/analyzers/exported-api.ts:627`, wrap package.json parsing in `try/catch` and continue to the next candidate on parse failure.
  • Readiness score is below the configured threshold — Use the readiness panel as advisory maintainer context; the score does not block this PR.
Review context
Contributor next steps
  • Explain no-issue PR.
  • Review top overlaps.
  • Add scope summary.
  • Fix blocker.
  • Expect slower review.
  • Refresh registry data or choose a registered active repo.
  • Link the issue being solved, or explicitly explain why this is a no-issue PR.
  • Check active issues and PRs before submitting.
Signal definitions
  • Related work = same linked issue, overlapping active PRs, or title/path similarity.
  • Review load = cached public PR metadata such as size labels, changed paths, and preflight status.
  • Open PR queue = repo-wide review pressure; it is not a PR quality failure.
  • Contributor context = public GitHub/Gittensor identity context; non-Gittensor status is not a blocker.
Review details

Generated from public PR metadata and the diff. Advisory only; deterministic signals remain authoritative.

The PR adds a new exported declaration-surface analyzer, wires it into brief generation, and covers the main tarball/patch/rendering paths. The overall shape is coherent, but the export parser misses a common declaration form and will silently under-report API removals for symbols declared with `export declare type` or `export declare interface`. Rendering and analyzer registration are straightforward.

Blockers

  • review-enrichment/src/analyzers/exported-api.ts:185 parses `export interface` and `export type` but not `export declare interface` or `export declare type`, so a published declaration like `export declare type Options = ...;` is omitted from the baseline surface and its removal will not be reported.

Nits (5)

  • review-enrichment/src/analyzers/exported-api.ts:627 should catch `JSON.parse(published.packageJson)` failures so one malformed tarball fails closed like the network and extraction paths instead of degrading the whole analyzer run.
  • review-enrichment/src/analyzers/exported-api.ts:465 relies on a line-oriented package.json overlay parser; add coverage for multi-line `exports` objects where unchanged entrypoints are outside the patch context so future edits do not turn partial diffs into false removals.
  • review-enrichment/src/analyzers/exported-api.ts:566 reports a missing head declaration file as `removed-entrypoint` even when the manifest still exposes the entrypoint but points at a missing types path; consider a more precise finding kind or wording to distinguish manifest removal from broken target files.
  • In `review-enrichment/src/analyzers/exported-api.ts:185`, change the interface/type matchers to accept optional `declare`, mirroring the function/class/enum cases, and add a regression test with `export declare type` plus removal.
  • In `review-enrichment/src/analyzers/exported-api.ts:627`, wrap package.json parsing in `try/catch` and continue to the next candidate on parse failure.

🟩 Safe / merged · 🟦 Advisory · 🟨 Held for review · 🟥 Blocked / closed


💰 Earn for open-source contributions like this. Gittensor lets GitHub contributors earn for the work they already do — register to start earning →.

Checked by Gittensory, a quiet PR intelligence layer for OSS maintainers.

  • Re-run Gittensory review

@EtoileAI

Copy link
Copy Markdown
Contributor Author

hi, @JSONbored. I wanted to contribute to this project based on your suggestion (I've already contributed to metagraphed-ui). Could you please review my PR?

@gittensory-orb gittensory-orb Bot added gittensor Gittensor contributor context gittensor:feature Gittensor-scored feature linked to a feature issue - worth 1.25x multiplier. labels Jun 29, 2026

@JSONbored JSONbored left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

Hi there, please review the gittensory review comment as it outlines some blockers. In addition to those blockers, there are also several merge conflicts, since a previous PR touching these same files was merged:

This branch has conflicts that must be resolved
Use the [web editor](https://github.com/JSONbored/gittensory/pull/1707/conflicts) or the command line to resolve conflicts before continuing.

review-enrichment/src/brief.ts
review-enrichment/src/render.ts
review-enrichment/src/types.ts

@JSONbored JSONbored closed this Jun 29, 2026
@github-project-automation github-project-automation Bot moved this from Todo to Done in gittensory - v1 roadmap Jun 29, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

gittensor:feature Gittensor-scored feature linked to a feature issue - worth 1.25x multiplier. gittensor Gittensor contributor context size:XL This PR changes 500-999 lines, ignoring generated files.

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

2 participants