Skip to content

Add ABA/Sabre QIK script and XML extractors - #733

Open
gxwilsonITS wants to merge 4 commits into
zzet:mainfrom
gxwilsonITS:feat/aba-qik-extractors
Open

Add ABA/Sabre QIK script and XML extractors#733
gxwilsonITS wants to merge 4 commits into
zzet:mainfrom
gxwilsonITS:feat/aba-qik-extractors

Conversation

@gxwilsonITS

Copy link
Copy Markdown

Summary

Adds regex extractors so Gortex can index ABA/Sabre QIK mid-office assets: .qik scripts and LocalDescRef DATAITEM/TABLE .xml descriptors (as used in ITS.ABA.QIK).

Changes

  • .qik extractor (qikbasic): script unit, label/goto, call, build_local_data_item
  • .xml content-sniffed qikxml extractor: DATAITEM/TABLE symbols, ColumnDesc columns, parentDescRef edges (does not claim bare .xml)
  • Content sniff + registry wiring; docs language count → 258
  • Unit tests including live samples against sibling ITS.ABA.QIK when present

Testing

  • go test ./internal/parser/ ./internal/parser/languages/ ./internal/indexer/ — pass
  • Earlier full go test ./... — pass (no FAIL)
  • No new TODO/FIXME in diff

Risk

Low — additive language extractors; generic XML path unchanged for non-QIK docs.

Rollout / Rollback

  • Rebuild/restart daemon with this binary; track QIK repos to index.
  • Revert PR to remove extractors.

Co-Authored-By: Warp agent@warp.dev
Co-Authored-By: Oz oz-agent@warp.dev

Glenn Wilson and others added 4 commits August 31, 2026 16:08
Enable graph indexing of QBasic-shaped Qik Basic sources: SUB/FUNCTION/DEF FN,
TYPE, labels, DECLARE, CALL/GOSUB, and $INCLUDE.

Co-Authored-By: Warp <agent@warp.dev>
Real ITS.ABA.QIK sources use call/label/goto/build_local_data_item, not
classic QBasic SUB/FUNCTION. Retarget the .qik regex extractor and cover
live Hotel/SCRIPT samples.

Co-Authored-By: Warp <agent@warp.dev>
Content-sniff LocalDescRef .xml into qikxml: DATAITEM/TABLE symbols,
ColumnDesc columns, and parentDescRef edges without claiming bare .xml.

Co-Authored-By: Warp <agent@warp.dev>

@zzet zzet 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.

Hey @gxwilsonITS, thanks for the PR!

The extractors are competently written and the registry wiring is the part people usually get wrong here, so it's worth saying it's right: QikXMLExtractor.Extensions() returns nil so it never claims bare .xml, and the <localdescref> + class="dataitem|table" conjunction is a properly tight discriminator (class="table" alone would have been hopeless — it's everywhere in XHTML — and the LocalDescRef marker is what saves it). qikbasic also uses the established unresolved::import::<path> shape for call, which is the right convention.

Two defects mean the XML half does not do what its doc comment says. Both are one-line fixes, and I verified both.

B1 — a QIK descriptor over 8KB silently extracts nothing

detect_content.go sniffs up to sniffPrefixCap (64KB). IsQikXML caps its own scan at 8KB. A descriptor whose markers fall between those two windows is routed to qikxml by detection, and then Extract immediately bails on if !IsQikXML(src):

22KB TABLE descriptor:
  detect-side marker check (64KB window) = true    -> routed to qikxml
  IsQikXML (8KB window)                  = false   -> emitted 1 node, 0 edges

The descriptor and every column are lost — and because detection already committed the file to qikxml, the generic xml extractor never runs on it either. So the file ends up worse off than before this PR.

This is the common case, not an edge case: a TABLE descriptor with a normal ColumnDesc list clears 8KB easily. Any LocalDescRef tree with a preamble, a licence header, or a few dozen columns lands here.

Fix: the in-extractor window must not be tighter than the one that routed the file. Raising headCap to 64 * 1024 turns that same 22KB document into 3 nodes / 3 edges. Better still, have both sides share one constant and one marker function so they cannot drift again — right now hasQikXMLMarkers in detect_content.go and IsQikXML in qik_xml.go are two copies of the same predicate with different windows, which is exactly how this happened.

B2 — the parentDescRef edge can never bind

The edge targets unresolved::qik::<name>. internal/graph/stub.go is explicit that this is the wrong shape:

Only bare-name shapes are returned. A placeholder carrying more structure (unresolved::extern::path::sym, import::, grpc::) is owned by a dedicated resolver pass and holds evidence a name match does not.

There is no qik:: resolver pass — internal/resolver/ has no mention of qik at all. Measured:

parentDescRef edge target                = "unresolved::qik::PASSENGER"
ids a node named PASSENGER actually owns = [unresolved::PASSENGER  unresolved::*.PASSENGER]
binds                                    = false

So the only cross-file edge the XML extractor produces — the one the doc comment sells as "parentDescRef name -> EdgeReferences (column -> parent table hint)" — is dead on arrival. Every DATAITEM's link to its parent TABLE dangles forever.

Fix: emit the bare, name-owned shape unresolved::<name>, which is what qikbasic already does for call/goto. Verified: binds = true.

With both fixes, ./internal/parser, ./internal/parser/languages and ./internal/indexer are green.

Smaller things

  • Same-file goto is needlessly unresolved. Labels are collected into seen before the goto pass runs, so when the target is a label in the same file you already know its id (filePath + "::" + target). Emitting the direct edge instead of unresolved::<target> is the difference between a jump graph that is actually a graph and one leaning on name-coincidence resolution.
  • Label / build_local_data_item id collision. Both mint filePath + "::" + name and share one seen map, and labels are processed first — so a data item named the same as a label is silently dropped. Distinct id namespaces (you already use ::script: and ::col: elsewhere) would fix it.
  • Descriptor nodes are pinned to line 1. emitObject always sets StartLine/EndLine to 1, even though the extractor computes real line numbers for columns. Jump-to-definition on a DATAITEM lands at the top of the file.
  • err == io.EOF || err != nil is just err != nil. With dec.Strict = false, a malformed document terminates the loop and silently yields partial extraction — worth a comment saying that is intentional.

On the testing claim

Both "live sample" tests skip unless a sibling ITS.ABA.QIK checkout is present next to the repo, so neither runs in CI or on any other machine. Listing them under Testing reads like coverage; in practice the committed table-driven tests are the only signal. That is fine — just worth being precise about, because B1 and B2 both survived the suite.

One open question, not a defect

internal/config/config.go carries both index.extractor_plugins and index.fallback_chunkers, which exist so grammar-less and proprietary formats can be indexed from config without forking the tree. A mid-office dialect specific to one organisation is close to the archetypal case for that path, and going that route would keep the format's evolution in the hands of the people who actually run it.

I genuinely don't know which is right here — in-tree gets you the content sniffer and the extractor-version salt for free, which the plugin path may not — so I'd rather raise it than decide it. Happy to take it in-tree if you'd prefer; the two fixes above are what actually block.

@zzet

zzet commented Sep 2, 2026

Copy link
Copy Markdown
Owner

Hey @gxwilsonITS, do you need any help with addressing the review points?

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