Skip to content

Feat/topological sort defs#242

Draft
vanshaj2023 wants to merge 2 commits into
ioflux-org:mainfrom
vanshaj2023:feat/topological-sort-defs
Draft

Feat/topological sort defs#242
vanshaj2023 wants to merge 2 commits into
ioflux-org:mainfrom
vanshaj2023:feat/topological-sort-defs

Conversation

@vanshaj2023

Copy link
Copy Markdown
Contributor

Summary

Implements topological sorting for $defs rendering order in sortAST.ts, and fixes an existing bug where modern $defs schemas received no sorting benefit at all.

Bug fixed: sortAST.ts only defined DEF_KEY (legacy definitions keyword). The modern $defs key (DEFS_KEY) was never referenced, meaning every schema using $defs got zero sorting benefit from the existing sort.

Problem: When $defs contains definitions that reference each other via $ref, rendering them out of dependency order produces spurious back-edges and a confusing graph layout. For example, if person references address but appears first in $defs, address gets rendered as a child of person and then again as a top-level definition.

Solution: Kahn's algorithm (BFS-based topological sort) — scans each definition's AST subtree for $ref pointers to sibling definitions, builds a dependency graph, and renders dependencies before dependents. Cycles are handled by appending remaining nodes in their original order — processAST already handles cycles safely via the renderedNodes Map.

What kind of change does this PR introduce

Bug fix + enhancement — fixes missing DEFS_KEY and adds topological sort for $defs rendering order

Issue Number

Closes #

Screenshots/Video

Beforeperson renders first, address becomes a child node then re-appears as top-level (spurious back-edge):

Aftercountry → address → person render in dependency order, clean left-to-right layout:

Schema used for testing:

{
  "$schema": "https://json-schema.org/draft/2020-12/schema",
  "$defs": {
    "person": {
      "type": "object",
      "properties": {
        "address": { "$ref": "#/$defs/address" }
      }
    },
    "address": {
      "type": "object",
      "properties": {
        "country": { "$ref": "#/$defs/country" }
      }
    },
    "country": {
      "type": "object",
      "properties": {
        "name": { "type": "string" }
      }
    }
  }
}
image

Does this PR introduce a breaking change?

No

If relevant, did you update the documentation?

No

@adityayadav-dev

Copy link
Copy Markdown
Contributor

Hey @vanshaj2023, since this falls under the GSoC qualification tasks, I believe it's intended to be part of the proposal rather than a PR—as mentioned in the task description. Opening a PR at this stage might create some unnecessary overhead for the maintainers. Thanks!

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