Add ABA/Sabre QIK script and XML extractors - #733
Conversation
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>
There was a problem hiding this comment.
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
gotois needlessly unresolved. Labels are collected intoseenbefore 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 ofunresolved::<target>is the difference between a jump graph that is actually a graph and one leaning on name-coincidence resolution. - Label /
build_local_data_itemid collision. Both mintfilePath + "::" + nameand share oneseenmap, 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.
emitObjectalways setsStartLine/EndLineto 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 != nilis justerr != nil. Withdec.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.
|
Hey @gxwilsonITS, do you need any help with addressing the review points? |
Summary
Adds regex extractors so Gortex can index ABA/Sabre QIK mid-office assets:
.qikscripts andLocalDescRefDATAITEM/TABLE.xmldescriptors (as used in ITS.ABA.QIK).Changes
.qikextractor (qikbasic): script unit,label/goto,call,build_local_data_item.xmlcontent-sniffedqikxmlextractor: DATAITEM/TABLE symbols,ColumnDesccolumns,parentDescRefedges (does not claim bare.xml)ITS.ABA.QIKwhen presentTesting
go test ./internal/parser/ ./internal/parser/languages/ ./internal/indexer/— passgo test ./...— pass (no FAIL)Risk
Low — additive language extractors; generic XML path unchanged for non-QIK docs.
Rollout / Rollback
Co-Authored-By: Warp agent@warp.dev
Co-Authored-By: Oz oz-agent@warp.dev