Plugin seams, the catalog, Mermaid diagrams — and a quality pass over all three - #2
Merged
Conversation
A seam is a contract plus the modules implementing it. It is the shape that makes a component both extractable and pluggable -- the same property read in two directions: if a group of modules depends on one declared contract and little else, it can be lifted out, and what comes out plugs back in elsewhere against that same contract. Measured, not guessed. Cohesion is |unit| / (|unit| + |escaping deps|), where the unit is the contract plus its implementers. On this repo, `extract/rewriters/base.py` scores 1.00 with zero escapes and `langs/base.py` scores 0.88 with one -- and `langs` is the component that was already extracted successfully into a standalone package, its single escape being exactly the module that extraction pulled in. The score predicted the outcome. Two distinctions decide whether the number means anything: Implementers versus consumers. Only modules declaring a type derived from the contract belong to the unit; modules that merely use it stay behind and keep depending on it, which is what a plug point is for. Counting consumers as members makes every popular interface look like a huge incohesive seam. The contract surface is every type the module declares, not only the abstract ones. These modules almost always ship a protocol alongside a convenience base that implements it, and implementers derive from the base -- matching only the protocol name found zero implementers for a package that plainly has seven. A contract can also be declared and not honoured: one real Go project has a plugin interface with 26 implementers and 45 escaping dependencies. That is reported as `entangled` rather than smoothed over, because the gap between a nominal plug point and a real one is the more useful fact. Adds the `plugin-seams` principle, `drydock extract --list-seams` to show cut points best-first with runnable commands, and `--seam CONTRACT` to extract one. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
34 tests. The two semantics that make the cohesion score mean anything are mutation-verified: Counting consumers as implementers fails the suite. Only modules declaring a type derived from the contract belong to the unit; modules that merely use it stay behind, which is what a plug point is for. Narrowing the contract surface to abstract types only fails the suite. These modules ship a protocol plus a convenience base, and implementers derive from the base -- matching only the protocol name finds no implementers at all. Also pinned: cohesion arithmetic on hand-checked fixtures, each verdict boundary, min_implementers, deterministic best-first ordering, base matching on the final dotted segment and through subscripted generics, Go's structural fallback as the documented imprecision it is, the principle degrading to None without a context rather than raising, --list-seams writing nothing, and --seam counting as a selection. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
docs/concepts/seams.md explains why extractable and pluggable are the same
structural property, and why the cohesion score predicts the outcome -- with
this repo's own langs extraction as the worked evidence.
Documents --seam and --list-seams, adds plugin-seams to the principles table,
and is honest about the limits: Go has no implements keyword so its
implementer detection is structural and imprecise, and a project may
legitimately have zero seams.
Replaces a LaTeX formula that rendered as raw `$$\text{...}$$` on the built
site; mkdocs has no math extension configured and adding one for a single
division is not worth the dependency.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
A PR stacked on another branch got no CI at all under `branches: [main]`, which is precisely the case where it matters most -- the stacked change is the one nobody has run yet. Adds workflow_dispatch for manual reruns. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
A user's accumulated library of ingested repositories, built data-layer-first so a CLI, a TUI, a GUI and an agent can all sit on the same API. Queries return plain dataclasses rather than database handles, so no frontend is coupled to how any of it is stored. SQLite through the stdlib `sqlite3`, which keeps the zero-dependency core intact while still giving real queries -- and asking questions across twenty ingested repositories at once is a query problem, not a file-walking one. FTS5 backs the question the catalog exists to answer: "do I already have something that does X?" Ids are content-addressed, so re-ingesting the same repository at the same commit updates rows instead of accumulating near-duplicates. This is a library added to over months; running it again has to be safe. Component code is copied into the catalog rather than referenced. A library of pointers into other people's checkouts stops working the moment a source moves, and you cannot compose from parts you do not have. Analyzer output is stored as JSON payloads rather than shredded into columns. Analyzers are plugins and their result shapes are theirs to change; only what the catalog itself ranks and filters on is promoted to real columns. Schema is versioned from the first write, and nothing is removed unless asked -- `forget()` reports what it deleted. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
`drydock catalog ingest` walks a project once and records what it finds: the repository at its commit, every architectural fact with its evidence, the components (seams where a codebase declares contracts, clusters where it does not, so a repository is never catalogued as empty merely for lacking Protocols), and the public symbols each component exposes. Capabilities are what make the library answerable. "Do I already have something that parses TOML?" is a question about symbols, not repositories, and it is the question composition depends on. Verified across five real projects: 23 components and 1,839 capabilities in five languages, searchable in one query. Mermaid because the same string renders in a GUI, on GitHub, in the docs site and in an artifact, and a model can read and write it. Fixes found while verifying the generated diagrams, all of which produced output that was wrong rather than merely ugly: Seam arrows pointed from the contract to its implementers, asserting that a contract depends on its own implementations -- the precise inversion a plugin architecture exists to avoid. The contract is a graph sink; every arrow now points at it. Escape edges were drawn as the cartesian product of implementers x escapes: 432 edges for a seam with 21 real ones, every fabricated one asserting a dependency the code does not have. Only real edges are drawn now, capped and counted. Package edges were emitted when *either* end survived the node cap, so a truncated diagram grew phantom unlabelled boxes where Mermaid invented the missing node. Both ends must now survive. Layer diagrams had no cap at all -- 1,079 lines for a 274-module project. Bounded like the others, with the omission stated in the diagram. The catalog CLI defaulted to human output behind a `--json` flag, which is the inverted convention 0.2 removed. It is JSON by default with `--markdown`, like every other command. Search now collapses hits that are the same thing seen twice: a file can belong to both a seam and an overlapping cluster, so its symbols were indexed once per component and came back repeated. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Validated against mermaid-cli 11.16 rather than a regex: 48 diagrams across four real projects and eleven adversarial fixtures now render, where before one did not and three more were wrong in ways no syntax check would catch. Labels used backslash escaping. Mermaid has no backslash escape -- `\"` is a parse error, not a quoted quote -- so a single directory named `quote"dir` produced an unrenderable chart. HTML entities now, verified both ways against the real parser. Escaping happened inside `_truncate_label`, so any caller that built a label another way skipped it silently. `package_diagram` did exactly that. Every node and subgraph now goes through one emission helper that escapes once; the call sites cannot forget because they no longer decide. Node ids collapsed distinct paths together: `a/b.py`, `a-b.py`, `a.b.py` and `a_b.py` all became `a_b_py`, so four modules rendered as one node with their edges merged. Nothing failed -- the diagram was simply a picture of a codebase that does not exist. Ids now carry a digest of the full path. Package labels emitted a literal `\n`, which Mermaid does not treat as a line break. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Every one of these produced a confident wrong answer from a typo, with exit
code 0 and output that looked entirely normal:
--max-files -5 analysed 0 files and reported truncated: true
--max-files 0 same
--top -1 listed 54 modules, because a Python slice of [:-1] is
"all but the last" rather than an error
--top 0 listed 15, because `int(opts.get("top") or 15)` treats 0 as
falsy and quietly substitutes the default
--days -30 returned status "ok" over a negative time window
`Option` now carries `minimum`/`maximum`, enforced once at the CLI boundary for
every analyzer rather than in each one, and the falsy-zero defaults are gone.
An out-of-range value is a usage error with a message that says the bound and
the value given.
Found by trying hostile inputs rather than by testing that valid ones work --
which is the same reason the diagram bugs survived a syntax check.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
45 tests. The diagram ones matter most: every bug they pin shipped, parsed cleanly, and was still wrong. Mutation-verified -- reverting the label escaping to backslashes fails four of them, removing the node-id digest fails another. Pinned: entity escaping with no ampersand (`&#quot;` renders a literal ampersand into the label, which is the bug), escaping applied at emission so no call site can skip it, node ids that never collide across `a/b.py` `a-b.py` `a.b.py` `a_b.py`, seam arrows pointing at the contract, no edge drawn that does not exist in the graph, bounded output with the truncation stated, and no edge referencing a node the cap removed. Catalog: content-addressed ids, upserts rather than duplicates, stored content surviving deletion of the source, `forget` leaving no orphaned rows (checked by direct SQL, not through the API that created them), malformed search returning empty rather than raising, and idempotent migrations. Every test uses a tmp_path catalog. Verified with a canary file that a full suite run leaves a real ~/.drydock byte-identical -- a test suite that could corrupt a user's accumulated library would be the worst defect in the project. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
`selection_type, _, selection_arg = active[0]` unpacked a `(is_set, name, value)` tuple with the flag bound to `selection_type`, so every subsequent `selection_type == "cluster"` compared a bool to a string and was always false. Every request fell through to the file-list branch, where a directory name was split on commas and matched nothing. Extraction over MCP -- the agent-facing half of the feature -- had therefore never worked. It reported `no files matched selection (True)`, and that stray `True` in a user-facing message was the only visible trace. Found by actually invoking all fifteen MCP tools rather than listing them. Listing had been the only check until now, and listing passes whatever the handler does. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Search could not answer the question the catalog exists for. Identifiers were indexed whole, and FTS5 tokenises on non-alphanumerics, so `BaseParser` was a single token and searching "parser" matched nothing. Identifiers are the data here and they are camelCase far more often than they are prose, so each one is now expanded into its words. Applied inside `_index_many` rather than at the call sites, because it was briefly applied to the single-row path only and capability symbols -- the ones that answer "do I have something that parses TOML?" -- silently missed out. Only files inside a detected component had their symbols indexed, and only about a quarter of a real project's modules fall inside one. Ingesting Drydock made 12 of its 46 modules searchable; a user looking for a module they had just ingested got nothing. Capabilities may now belong to no component (schema v2, migrating existing catalogs without data loss), and ingest indexes the remainder. Drydock now yields 74 searchable modules and 1,010 symbols rather than 12 and 55. Two bugs found while adding that, both silent: `component_id = NULL` never matches in SQL, so the replace-on-reingest path did nothing for unowned capabilities. Now `IS ?`. The delete was not scoped by repository, so `component_id IS NULL` matched every repo's unowned symbols and ingesting one project deleted another's. Capability ids likewise omitted the repo, so the same symbol in two projects collided into one row. Components are also named after the package holding the contract rather than the filename: contract modules are nearly always `base.py` or `types.go`, so a listing showed several components all called "base". Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Lumi-node
force-pushed
the
feat/plugin-seams
branch
from
August 22, 2026 22:34
02cf116 to
b684b95
Compare
Lumi-node
changed the title
None of these were caught by the tests written alongside the feature, because each returned a well-formed answer -- usually an empty one. Mutation-verified: reverting any of the three fixes fails a test here. A camelCase identifier is one FTS5 token, so `parser` did not find `BaseParser` and the catalog could not answer the reuse question it exists for, while searches for whole identifiers still worked and hid it. The expansion was applied on the single-row index path only, so component names became searchable by word and capability symbols did not. Capabilities required a component, and only about a quarter of a project's modules are in one, so most of an ingested repository was invisible. `component_id = NULL` is never true in SQL, so re-ingesting never cleared the previous unowned set. That delete was not scoped by repository either, so ingesting one project silently deleted another's symbols -- data loss in the asset the product is built on. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…in surface Answers the questions a vulnerability scanner cannot, because they need a resolved dependency graph rather than a package manifest: who can reach the code that handles credentials, can a module accepting outside input reach a subprocess call, and how much third-party code does the sensitive path drag in. Deliberately not a vulnerability scanner. Snyk, Semgrep and GHAS match CVEs better than this would, and duplicating them adds noise without information. **Modules are classified by what they import, never by what they are named.** A file called `auth.py` importing nothing sensitive is not sensitive; a `helpers.py` importing `subprocess` and `pickle` is. Every classification names the exact import that produced it, and every path finding shows the actual import chain rather than asserting one exists. Two precision decisions, because a security report that cries wolf teaches people to ignore it: Hashing is a separate, weaker class from secrets. `hashlib` is as often a checksum as a credential -- this project's own diagram module hashes file paths to build node ids, and calling that "handles secrets" would discredit every other finding in the report. Test modules are excluded by default. They are not deployed, and including them filled the findings with `*_test.go`. Stated in the output, not just here: this analyses imports, not data flow. A reported path exists in the dependency graph; it is not evidence that untrusted input reaches the call. Findings are places to look. Discriminates correctly across real projects: this repo and a 272-module Python project show zero ingress (neither is a server), while two Go services surface genuine ingress-to-execution paths. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The last two undocumented surfaces. catalog.md covers all eight sub-commands with real output, the Python API a TUI or GUI would build on, and the five MCP tools -- noting that `drydock_catalog_ingest` writes to the user's library, unlike every analyzer tool. diagrams.md documents the four diagram kinds and, more usefully, the constraints that make them trustworthy: HTML-entity escaping because Mermaid has no backslash escape, node ids carrying a path digest because four distinct paths otherwise collapse into one node, no edge drawn that does not exist, and bounded output with truncation stated rather than applied silently. Each of those was a real bug, and saying so is the point. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…usly 35 tests for architectural security, pinning the properties the report's credibility rests on: classification by import and never by name (mutation: classifying by filename fails 12 tests), evidence naming the exact import, hashing kept separate from secrets, test modules excluded by default, both traversal directions, cycle-safety, determinism, and the limitation string always present in the output. The path-reality test needed strengthening before it was worth anything. It wrapped its assertions in `if dangerous:`, so it passed whenever no finding was produced, and its fixture had a one-hop route -- which meant a deliberately fabricated direct edge coincided with a real one and could not be detected. It now asserts the finding exists and uses a two-hop route, and a fabricating implementation fails it. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Every assertion in these sat behind an `if`, so each passed whenever its condition was false -- reporting success without checking anything. The same failure mode this project keeps finding in its own output, relocated into the test suite. One of them had already passed against an implementation deliberately rewired to fabricate paths. Fixed by asserting the precondition rather than deleting the conditional: if a test's assertions only run when some state holds, that state holding is part of what the test asserts. Three had been "fixed" by weakening instead, and are restored: `test_stable_dependencies_violations_populated` had been reduced to "a finding exists with evidence", which would pass against an implementation that never populates violations at all. Making it assert properly revealed the fixture pointed every edge from unstable to stable -- the principle HOLDING -- so it could never have observed a breach. The fixture now builds a stable module depending on a volatile one, and the test checks the offender really does depend toward less stability. `test_no_packaging_overwrites_plan_files` had been reduced to asserting some files were generated, leaving the overwrite protection its name describes entirely untested. It now puts a pyproject.toml in the plan and asserts none is generated; removing the guard fails it. `test_list_seams_ordered_best_first` looped over a list of one, so the ordering assertion never ran. It has its own fixture now with two seams of differing cohesion. Zero `if`-guarded tests remain. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.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
1 participant
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.
Three things, plus the quality pass that followed them. Rebased onto
mainnow that #1 has landed.Plugin seams
A seam is a contract plus the modules implementing it — the shape that makes a component both extractable and pluggable, which are the same structural property read in two directions.
Cohesion is
|unit| / (|unit| + |escaping deps|). On this repoextract/rewriters/base.pyscores 1.00 with zero escapes andlangs/base.pyscores 0.88 with one — andlangsis the component already extracted in #1 into a pip-installable package, whose single escape was exactly the module that extraction pulled in. The score described the outcome before anything was cut.drydock extract --list-seamsranks cut points and emits runnable commands;--seam CONTRACTcuts one.Reported honestly when a contract is declared but not honoured: one real Go project has a plugin interface with 26 implementers and 45 escaping dependencies, which is
entangled, not a seam.The catalog
A user's accumulating library, built data-layer-first so a CLI, TUI, GUI and agent share one API. SQLite via the stdlib, FTS5 for search, content-addressed ids so re-ingest updates rather than duplicates, and component code copied in so the library survives its source moving.
Verified across five real projects: 23 components and 1,839 capabilities in five languages, searchable in one query.
Mermaid diagrams
Layer, package, seam and component diagrams. 48/48 render through mermaid-cli 11.16, including eleven adversarial fixtures (unicode filenames, quotes in directory names, files named
graph TD.py, symlink loops, binary files, non-UTF8 encodings).The quality pass
Every bug below shipped, passed a syntax or happy-path check, and was still wrong. That failure mode — confident, plausible, incorrect — is the one this pass targeted.
Diagrams
\"is a parse error). One directory namedquote"dirproduced an unrenderable chart.package_diagram— which built labels its own way — skipped it entirely.a/b.py,a-b.py,a.b.py,a_b.pyinto one node with merged edges. Nothing failed; the diagram was a picture of a codebase that does not exist.CLI
--max-files -5analysed zero files and reportedtruncated: true, exit 0.--top -1listed all-but-one via a Python slice.--top 0silently meant 15, becauseor 15treats zero as falsy. Numeric options are now bounded at the shared CLI boundary.summary— the one key common to all of them.MCP
(is_set, name, value)tuple was unpacked with the flag bound to the name, so every comparison compared a bool to a string. Found by invoking all fifteen tools rather than listing them.Catalog
BaseParseris one FTS5 token, so searchingparsermatched nothing; identifiers are now expanded into their words.component_id = NULLnever matches in SQL, so the replace-on-reingest path did nothing for unowned capabilities; and the delete was not repo-scoped, so ingesting one project deleted another's symbols.Verification
474 tests, from 353. Key contracts are mutation-tested — reverting the label escaping fails four, removing the node-id digest fails one. A canary check proves a full suite run leaves a real
~/.drydockbyte-identical. CI now runs on every PR, not only those targetingmain, which is why this one is checked at all.🤖 Generated with Claude Code