fix(service-patterns): match library ids on identifier boundaries - #2273
Merged
Merged
Conversation
Distilled from #1245 by Andrew Hundt (d15071c, the match_qn half). match_qn was a raw strstr(qn, library_id). Short ids fired inside unrelated words, and a service kind REPLACES the plain CALLS edge, so a false match both invents a service edge and deletes a real call: "gin." in plugin. -> route registration; any "/"-prefixed first argument then minted a Route node "nconf" in encryptionconfig -> CONFIGURES for every call into the package "tonic" in Monotonic -> gRPC; unparseable, so NO edge at all "express" in expression -> route registration "get_env" in get_envelope, get_environ -> CONFIGURES "dio" "surf" "curl" "resty" "phin" "hyper" "treq" "rocket" in studio, surface, curly, restyle, dolphin, hyperbolic, streq, rocketmq The rule (qn_hit_on_boundary; every occurrence of an id is tried, not only the first). A separator is any character that is not an ASCII letter or digit. before: start of string | separator | the id itself starts with a separator ("@trpc/server") | the id starts with an uppercase letter -- a capital opens a new CamelCase word whatever precedes it (AsyncHttpClient, IHttpClientFactory, NSURLSession) after: end of string | separator | the id itself ends in a separator ("gin.") | uppercase letter (GuzzleHttp, FeignClient, KafkaProducer) | digit (urllib2, Mint.HTTP2, amqp091-go) Rejected: a lowercase- or digit-initial id glued to a preceding letter or digit, and any id continued by a lowercase letter. Two deliberate differences from the upstream hunk: upstream's before-rule needed a GuzzleHttp table entry and would have dropped IHttpClientFactory and NSURLSession, and without the digit after-boundary urllib2 and amqp091-go stop matching. All three match on main today. No recall is traded for this. No boundary rule can tell "grequests" from "myrequests", so a library whose own name glues a prefix or suffix onto another id gets an explicit entry with the kind (and broker) it had on main -- 47 of them, found mechanically by running main's matcher and the new one over 173 candidate names and listing every one that went from a kind to NONE: HTTP grequests txrequests redaxios gaxios libcurl curlpp curlcpp hyperlocal guzzlehttp ASYNC aiokafka pykafka rskafka librdkafka rdkafkacpp aioamqp amqpstorm pamqp pyamqp amqprs amqpcpp pynats jnats aiomqtt amqtt hbmqtt umqtt mqttools emqtt rumqtt gomqtt libmosquitto mosquittopp pubsublite gocelery CONFIG wgetenv getenvb qgetenv dotenvx phpdotenv ROUTE_REG apiflask honox fasthttprouter GRAPHQL gqlparser gqlgenc aiogqlc Left at NONE on purpose: names that are not clients of the id they contain and were misclassified on main -- hypercorn, hyperlink, hyperopt, openresty, gqlalchemy, celeryconfig, kafkacat, gnatsd, pypubsub. One visible reclassification: hyperium/tonic was HTTP through "hyper" inside "hyperium" and is now gRPC through "tonic". RED, production reverted and the final tests kept: infrascan_service_pattern_match_rejects_ids_inside_words FAIL tests/test_infrascan.c:105: svc_case_mismatches(cases) == 28, expected 0 == 0 4 passed, 1 failed All 28 negatives mismatch on main. The 47 glued-library positives are RED the other way round: against the boundary rule without the entries. Each positive uses a name containing no second id, so it binds to its own entry -- "aiokafka.AIOKafkaProducer" would have passed through "KafkaProducer" and proven nothing. GREEN: infrascan 5, pipeline 281, parallel 74, extraction 350, edge_types_probe 59, route_canon 11, lang_contract 41, cross_repo 8, registry 64, mcp 318 (4 Windows-only skips). 0 failed. Effect on real code -- production binaries, main 92abefa against this change. main against main differs by 0 edges and 0 nodes on all three corpora, so every delta below is the change: django CONFIGURES 76 -> 74; both calls return as CALLS (geom.get_envelope, WSGIRequestHandler.get_environ) typescript CONFIGURES 12,026 -> 12,024; one bogus Route "/*type*/" gone with its CALLS and HANDLES; 3 calls return as CALLS kubernetes CONFIGURES 7,962 -> 7,609: all 353 are "nconf" inside encryptionconfig / authorizationconfig / authenticationconfig, and all 353 return as CALLS. 8 bogus Routes gone -- os.ReadFile of the serviceaccount namespace file, t.Run("/TwoWay"), "/tmp/test" -- with 26 CALLS, 2 HANDLES and 21 TESTS edges that pointed at them. 374 plain CALLS restored; 8 of them (validate.Monotonic and friends) had NO edge at all on main. GRPC_CALLS 60 -> 60. HTTP_CALLS 398 -> 407: nine calls main swallowed as route registrations now reach the existing arg_url heuristic like every other package. 88 of the 408 removed service-derived edges were read at the call site -- all of django and typescript, 82 of 402 in kubernetes: 88 false positives, 0 true positives lost. None of the three corpora uses a glued-name library, so the 47 entries are covered by the unit test only. Known limits: "_" is a separator, so optical_fiber.len still matches "fiber." exactly as requests_get matches "requests" by design; a lowercase id followed by a capital matches (kafkaProducer), which also admits a dioXide-style name; a user wrapper glued without a separator (mygetenv) is no longer classified; a glued-name library missing from the list is NONE until someone adds it. Co-authored-by: Andrew Hundt <ATHundt@gmail.com> Signed-off-by: Martin Vogel <martin.vogel.tech@gmail.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Service-pattern library ids matched inside unrelated words, inventing service edges and deleting real CALLS. Distilled from #1245 (Andrew Hundt, the
match_qnhalf of d15071c), re-implemented against today's tree.The defect
match_qnwas a rawstrstr(qn, library_id). Short ids fired inside unrelated identifiers, and because a service kind replaces the plain CALLS edge, every false match both minted a bogus service edge and removed a true call:gin.plugin./-prefixed first argument minted a Route nodenconfencryptionconfigtonicMonotonicexpressexpressionget_envget_envelope,get_environdiosurfcurlrestyphinhypertreqrocketstudio,surface,curly,restyle,dolphin,hyperbolic,streq,rocketmqThe rule
qn_hit_on_boundary: every occurrence of an id is tried, and a hit needs an identifier boundary on both sides. A separator is any character that is not an ASCII letter or digit.@trpc/server), or an id starting with an uppercase letter — a capital opens a new CamelCase word whatever precedes it (AsyncHttpClient,IHttpClientFactory,NSURLSession)gin.), an uppercase letter (GuzzleHttp,FeignClient,KafkaProducer) or a digit (urllib2,Mint.HTTP2,amqp091-go)Two deliberate differences from the upstream hunk: upstream's before-rule needed a
GuzzleHttptable entry and would have droppedIHttpClientFactoryandNSURLSession; without the digit after-boundary,urllib2andamqp091-gostop matching. All three match onmaintoday.No recall traded
No boundary rule can tell
grequestsfrommyrequests, so every real library whose own name glues a prefix or suffix onto another id gets an explicit table entry with the kind it had onmain— 47 of them, found mechanically by runningmain's matcher and the new one over 173 candidate names and listing every one that went from a kind to NONE. Names that were misclassified onmain(hypercorn,hyperlink,openresty,celeryconfig,kafkacat, …) are deliberately left at NONE. One visible reclassification:hyperium/tonicwas HTTP throughhyperinsidehyperiumand is now gRPC throughtonic.RED → GREEN
RED with production reverted and the final tests kept:
infrascan_service_pattern_match_rejects_ids_inside_wordsfails with 28 mismatches. The 47 glued-library positives are RED the other way round (boundary rule without the entries); each positive uses a name containing no second id, so it binds to its own entry.GREEN on today's
main: infrascan 5, pipeline 286, extraction 371, plus parallel, edge_types_probe, route_canon, lang_contract, cross_repo, registry and mcp; memory-core linter unchanged.Effect on real code (production binaries,
mainvs this change;mainvsmaindiffers by 0)/*type*/gone with its CALLS and HANDLES; 3 calls return as CALLSnconfinsideencryptionconfig/authorizationconfig/authenticationconfig, all 353 return as CALLS. 8 bogus Routes gone (os.ReadFileof the serviceaccount namespace file,t.Run("/TwoWay"),"/tmp/test") with 26 CALLS, 2 HANDLES and 21 TESTS edges that pointed at them. 374 plain CALLS restored, 8 of which had no edge at all onmain. GRPC_CALLS 60 → 60. HTTP_CALLS 398 → 40788 of the 408 removed service-derived edges were read at the call site — all of django and typescript, 82 of 402 in kubernetes: 88 false positives, 0 true positives lost.
Known limits
_is a separator, sooptical_fiber.lenstill matchesfiber.exactly asrequests_getmatchesrequests, by design. A lowercase id followed by a capital matches (kafkaProducer), which also admits adioXide-style name. A user wrapper glued without a separator (mygetenv) is no longer classified. A glued-name library missing from the list is NONE until someone adds it.