diff --git a/.docket.toml b/.docket.toml index b2890d0..e325f59 100644 --- a/.docket.toml +++ b/.docket.toml @@ -12,6 +12,10 @@ doneDir = "done" defaultPriority = 2 maxPriority = 4 +# How many nodes `docket docs roadmap` aims to draw. Past this it drops the completed tickets furthest from the work still open. +# Set it to 0 for no ceiling, and remember that mermaid stops being readable well before a thousand nodes. +maxRoadmapNodes = 200 + # How long, in seconds, one docket process waits for another to finish writing before giving up. # Writes are serialized across processes, so a CLI command and a running MCP server never overwrite each other. lockTimeout = 5.0 diff --git a/README.md b/README.md index 449caac..fe55ae7 100644 --- a/README.md +++ b/README.md @@ -101,9 +101,14 @@ docket list [-s todo] [-k CORE] [-m 2] [-r] docket graph [-i CORE-14 | -k GEN | -s todo] [-o FILE] docket key list | add KEY "desc" [-r TEXT] | remove KEY docket validate | deploy PATH | upgrade PATH -docket docs handoff [-o FILE] +docket docs handoff [-p] [-o FILE] +docket docs roadmap [CORE-14 | GEN | todo] [-m N] [-p] [-o FILE] ``` +`docs` writes a file: `handoff.md` and `roadmap.md` at the repo root. `-p/--print` sends it to stdout instead, `-o` picks another path, and passing both writes the file and prints it. `graph` is the exception, printing unless you ask for a file, because its output is usually piped. + +`docket docs roadmap` is the committable picture of the graph: a markdown file wrapping a mermaid diagram, plus the legend a renderer that ignores styling would otherwise leave you without. Past `maxRoadmapNodes` it drops the completed tickets furthest from the work still open, and never drops an open ticket, so the diagram stays readable without losing what is ahead. + `-r` replaces the dependency list. `-ra` and `-rr` edit the one already there. Both in one call is refused. A ticket is ready when every id in its `requires` names a ticket that is `done`. A missing dependency blocks, and a `done` ticket is never ready, so `docket list -r` is the set you can pick up right now. @@ -157,6 +162,7 @@ doneDir = "done" defaultPriority = 2 maxPriority = 4 lockTimeout = 5.0 +maxRoadmapNodes = 200 [keys] # Primary arch @@ -167,6 +173,8 @@ META = "campaign and progression" A key's rationale becomes the comment above it, and removing the key takes the comment with it. The status vocabulary is deliberately not configurable. +`maxRoadmapNodes` is how many nodes `docket docs roadmap` aims to draw, and `0` turns the ceiling off. + `docket deploy` never rewrites an existing `.docket.toml`. Run `docket upgrade .` later to refresh the template and repair the server entry without touching your config or tickets. ## Concurrent Access diff --git a/docs/tickets/todo/FEAT-14_roadmapGraphGeneration.md b/docs/tickets/done/FEAT-14_roadmapGraphGeneration.md similarity index 93% rename from docs/tickets/todo/FEAT-14_roadmapGraphGeneration.md rename to docs/tickets/done/FEAT-14_roadmapGraphGeneration.md index 61457f8..fc9989a 100644 --- a/docs/tickets/todo/FEAT-14_roadmapGraphGeneration.md +++ b/docs/tickets/done/FEAT-14_roadmapGraphGeneration.md @@ -1,9 +1,9 @@ --- id: FEAT-14 title: Roadmap Graph Generation -status: todo +status: done priority: 2 -requires: [FEAT-6] +requires: [] metadata: {} --- diff --git a/roadmap.md b/roadmap.md new file mode 100644 index 0000000..9c76cee --- /dev/null +++ b/roadmap.md @@ -0,0 +1,78 @@ +# Roadmap + +> Generated with `docket docs roadmap`. + +```mermaid +graph TD + subgraph BUG + BUG_1("BUG-1
Fix Character Encoding
Issue in FEAT-5
p1 done") + BUG_2("BUG-2
Cannot Clear with Set
Command
p0 done") + BUG_3("BUG-3
Migrate Server to MCP
2.x MCPServer API
p0 done") + BUG_4("BUG-4
Add to Requires in CLI
p1 done") + BUG_5("BUG-5
Serialize Ticket Writes
Across Processes
p3 done") + BUG_6["BUG-6
Key Removal Checks Usage
Outside the Lock
p4 todo"] + end + subgraph FEAT + FEAT_1("FEAT-1
Record Demo GIF with VHS
p2 done") + FEAT_2("FEAT-2
Set Up and Publish to
PyPI
p3 done") + FEAT_3("FEAT-3
All Tickets Graph
p2 done") + FEAT_4("FEAT-4
Shorthand for CLI
p2 done") + FEAT_5("FEAT-5
Selection Options in CLI
p2 done") + FEAT_6["FEAT-6
Templates for Tickets
per Key
p3 todo"] + FEAT_7("FEAT-7
Unified Version Code
p1 done") + FEAT_8["FEAT-8
Host Flag
p4 todo"] + FEAT_9("FEAT-9
Track Arbitrary
Additional Metadata on
Tickets/Groups
p1 done") + FEAT_10("FEAT-10
Tree Style CLI Commands
p2 done") + FEAT_11("FEAT-11
Repository Automation
and Contribution
Scaffolding
p3 done") + FEAT_12["FEAT-12
Per Key Ticket Board
View
p2 todo"] + FEAT_13("FEAT-13
Check if Ticket Is Ready
for Work
p1 done") + FEAT_14["FEAT-14
Roadmap Graph Generation
p2 todo"] + FEAT_15("FEAT-15
Use Title Case for
Tickets
p1 done") + FEAT_16["FEAT-16
Ticket Show Uses
Optional Formatting
p2 todo"] + FEAT_17("FEAT-17
Add Status to Graph
Scope
p1 done") + FEAT_18("FEAT-18
Offsite Ticket Authoring
Brief
p1 done") + FEAT_19["FEAT-19
Add Google Style
Documentation Comments
p1 todo"] + end + BUG_1 --> FEAT_2 + BUG_1 --> FEAT_5 + BUG_2 --> FEAT_2 + BUG_3 --> FEAT_2 + BUG_4 --> FEAT_2 + BUG_5 --> BUG_6 + BUG_5 --> FEAT_2 + FEAT_1 --> FEAT_2 + FEAT_2 --> FEAT_11 + FEAT_4 --> FEAT_2 + FEAT_5 --> FEAT_2 + FEAT_6 --> FEAT_12 + FEAT_7 --> FEAT_2 + FEAT_10 --> FEAT_2 + classDef doneP0 fill:#2d6a4f,color:#fff,stroke:#ff6b6b,stroke-width:4px + classDef doneP1 fill:#2d6a4f,color:#fff,stroke:#ff922b,stroke-width:3px + classDef doneP2 fill:#2d6a4f,color:#fff,stroke:#ffd43b,stroke-width:2px + classDef doneP3 fill:#2d6a4f,color:#fff,stroke:#adb5bd,stroke-width:2px + classDef todoP1 fill:#495057,color:#fff,stroke:#ff922b,stroke-width:3px + classDef todoP2 fill:#495057,color:#fff,stroke:#ffd43b,stroke-width:2px + classDef todoP3 fill:#495057,color:#fff,stroke:#adb5bd,stroke-width:2px + classDef todoP4 fill:#495057,color:#fff,stroke:#6c757d,stroke-width:1px + class BUG_2,BUG_3 doneP0 + class BUG_1,BUG_4,FEAT_13,FEAT_15,FEAT_17,FEAT_18,FEAT_7,FEAT_9 doneP1 + class FEAT_1,FEAT_10,FEAT_3,FEAT_4,FEAT_5 doneP2 + class BUG_5,FEAT_11,FEAT_2 doneP3 + class FEAT_19 todoP1 + class FEAT_12,FEAT_14,FEAT_16 todoP2 + class FEAT_6 todoP3 + class BUG_6,FEAT_8 todoP4 +``` + +## Legend + +| Shape | Status | +|---|---| +| `[ ]` | todo, not started | +| `{ }` | wip, in flight | +| `( )` | done | + +Arrows indicate required order of operations. + +Each node's border represents its priority. Heaviest is higher priority. diff --git a/src/docket/__init__.py b/src/docket/__init__.py index a47f76a..799bdaf 100644 --- a/src/docket/__init__.py +++ b/src/docket/__init__.py @@ -8,4 +8,4 @@ # No type check to comply with hatch's requirements. # Do not re-add. -__version__ = "1.4.0" +__version__ = "1.5.0" diff --git a/src/docket/cli/__init__.py b/src/docket/cli/__init__.py index 62e7319..d0041dd 100644 --- a/src/docket/cli/__init__.py +++ b/src/docket/cli/__init__.py @@ -20,13 +20,16 @@ commandList, commandMeta, commandNew, + commandRoadmap, commandSet, commandShow, commandStatus, commandStatusRead, commandTicket, commandValidate, + documentPath, emitDocument, + requireScopeKey, ) from docket.cli.grammar import ( CLEAR_SENTINEL, @@ -40,6 +43,7 @@ TOKEN_KEY, TOKEN_PRIORITY, TOKEN_STATUS, + addScopeArguments, buildParser, classifyToken, describeKeys, @@ -73,6 +77,7 @@ "TOKEN_PRIORITY", "TOKEN_STATUS", "Output", + "addScopeArguments", "buildContextTable", "buildParser", "classifyToken", @@ -83,6 +88,7 @@ "commandList", "commandMeta", "commandNew", + "commandRoadmap", "commandSet", "commandShow", "commandStatus", @@ -92,11 +98,13 @@ "describeKeys", "describePriorities", "dispatch", + "documentPath", "emitDocument", "main", "parseEditIdList", "parseIdList", "relativeToRoot", + "requireScopeKey", "resolveGraphScope", "resolveListFilters", "rewriteIdFirst", diff --git a/src/docket/cli/commands.py b/src/docket/cli/commands.py index e777443..7b0b6b1 100644 --- a/src/docket/cli/commands.py +++ b/src/docket/cli/commands.py @@ -17,12 +17,13 @@ from docket.cli.grammar import EXIT_INVALID, EXIT_OK, EXIT_USAGE, OUTPUT_ARGUMENT, parseEditIdList, parseIdList, resolveGraphScope, resolveListFilters from docket.cli.output import STATUS_STYLES, Output, buildContextTable, relativeToRoot -from docket.core.config import Config +from docket.core.config import Config, discoverConfig from docket.core.deploy import DeployReport, deploy, upgrade -from docket.core.handoff import renderHandoff -from docket.core.graph import Readiness, ResolvedGraph, dependencyContext, readyTickets, resolveGraph, subgraphForId, subgraphForKey, subgraphForStatus, ticketReadiness +from docket.core.handoff import HANDOFF_FILENAME, renderHandoff +from docket.core.graph import Readiness, ResolvedGraph, dependencyContext, readyTickets, resolveGraph, scopeGraph, ticketReadiness from docket.core.inputs import requireWritableFile, writeFile from docket.core.mermaid import renderGraph +from docket.core.roadmap import ROADMAP_FILENAME, Roadmap, buildRoadmap from docket.core.store import Store, TicketResult, TicketSet from docket.core.ticket import STATUSES, Ticket from docket.core.validate import SEVERITY_ERROR, ValidationReport, validate @@ -329,18 +330,9 @@ def commandGraph(args: argparse.Namespace, store: Store, output: Output) -> int: ticketId, key, status = resolveGraphScope(args.scope, args.id, args.key, args.status) - graph: ResolvedGraph = resolveGraph(store.loadAll()) + requireScopeKey(store, key) - # Scope the graph when asked. At most one of the three survives resolution. - if ticketId is not None: - graph = subgraphForId(graph, ticketId) - elif key is not None: - # For the same reason as `list`, an unregistered key here would render an empty graph rather than admitting the key does not exist. - store.config.requireKnownKey(key) - graph = subgraphForKey(graph, key) - elif status is not None: - # No equivalent check, because the vocabulary is fixed and both spellings are checked against it before they arrive. An empty result here is a true answer rather than a typo. - graph = subgraphForStatus(graph, status) + graph: ResolvedGraph = scopeGraph(resolveGraph(store.loadAll()), ticketId, key, status) source: str = renderGraph(graph) @@ -421,36 +413,75 @@ def commandValidate(args: argparse.Namespace, store: Store, output: Output) -> i return EXIT_INVALID if report.errors else EXIT_OK -def emitDocument(text: str, destination: Optional[str], name: str, output: Output) -> int: +def requireScopeKey(store: Store, key: Optional[str]) -> None: + """ + Refuse a scope naming a key the repository has not registered. + + Scoping to an unknown key would draw an empty graph, which reads as an answer rather than as the typo it is. `list` refuses one for the same reason. A status needs no equivalent check, since the vocabulary is fixed and both spellings are checked against it before they arrive here, so an empty result there is a true answer. + + store: The store holding the registry. + key: The key the scope named, or `None` when it named something else. """ - Write rendered text to a file when one was named, and to stdout when one was not. - Both the mermaid source and the shipped documents are text a machine reads next, so both leave through here rather than each growing their own copy of the rule. + if key is not None: + store.config.requireKnownKey(key) + + +def documentPath(config: Optional[Config], filename: str) -> Path: + """ + Place a shipped document's prescribed destination. + + A document belongs to the repository it describes, so it lands beside the configuration that governs it. Run outside a repository there is no such place, and the working directory is the only honest fallback, which is the same reasoning that lets the brief render without a configuration at all. + + config: The configuration governing the document, or `None` when none was found. + filename: What the document is called. + + Returns the path to write to unless the caller names another. + """ + + return (config.repoRoot if config is not None else Path.cwd()) / filename + + +def emitDocument(text: str, destination: Optional[str], name: str, output: Output, defaultPath: Optional[Path] = None, toPrint: bool = False, note: str = "") -> int: + """ + Send rendered text to the file it belongs in, to stdout, or to both. + + Every command that renders text leaves through here rather than each growing its own copy of the rule. What differs between them is only whether they have a file to fall back on: a document does and so writes one unasked, while `graph` does not and so stays a pipe. + + A named destination always wins. Without one, the prescribed path is written unless printing was asked for instead, and asking for both does both. text: The rendered text to emit. - destination: The path to write to, or `None` to write to stdout. + destination: The path the caller named, or `None` when none was named. name: What to name the destination in an error message, for example `--output path`. output: Where to write. + defaultPath: The path to write when none was named, or `None` to leave stdout as the only destination. + toPrint: Whether to print the text to stdout. + note: Anything to append to the confirmation line, already spaced and parenthesized. Returns the process exit code. """ - if destination is not None: - # Check the destination before rendering work is spent on it, and translate whatever the filesystem still refuses, so no write failure reaches the user as a traceback. - outPath: Path = writeFile(requireWritableFile(destination, name), text, name) - output.print(f"Wrote {outPath}") + chosen: Optional[str] = destination - return EXIT_OK + # Fall back to the prescribed path, which printing replaces rather than adds to, so a bare print stays clean enough to pipe. + if chosen is None and defaultPath is not None and not toPrint: + chosen = str(defaultPath) + + if chosen is not None: + # Check the destination before the filesystem is touched, and translate whatever it still refuses, so no write failure reaches the user as a traceback. + outPath: Path = writeFile(requireWritableFile(chosen, name), text, name) + output.print(f"Wrote {outPath}{note}") - # Straight to stdout with no styling, so a redirect captures exactly what was rendered and nothing else. - output.raw(text) + # Straight to stdout with no styling, so a redirect captures exactly what was rendered and nothing else. With nothing written this is the whole of the command's output. + if toPrint or chosen is None: + output.raw(text) return EXIT_OK def commandDocs(args: argparse.Namespace, config: Optional[Config], output: Output) -> int: """ - Print a document docket ships, rendered for this repository. + Write a document docket ships, rendered for this repository. args: The parsed arguments. config: The configuration governing the current directory, or `None` when none was found. @@ -459,14 +490,45 @@ def commandDocs(args: argparse.Namespace, config: Optional[Config], output: Outp Returns the process exit code. """ - if args.docsCommand != "handoff": - output.error("Expected one of: handoff.") - return EXIT_USAGE + if args.docsCommand == "handoff": + # A configuration is what lets the brief name real keys and real numbering, but its absence is a state the document handles rather than an error, since a person may be anywhere when they go to fetch it. + store: Optional[Store] = Store(config) if config is not None else None + + return emitDocument(renderHandoff(store), args.output, OUTPUT_ARGUMENT, output, documentPath(config, HANDOFF_FILENAME), args.toPrint) + + if args.docsCommand == "roadmap": + # The roadmap is nothing but this repository's own tickets, so unlike the brief it cannot be rendered without one. Discovery is repeated here so the reason a configuration could not be found is reported by the code that knows it. + return commandRoadmap(args, Store(config if config is not None else discoverConfig()), output) + + output.error("Expected one of: handoff, roadmap.") + + return EXIT_USAGE + + +def commandRoadmap(args: argparse.Namespace, store: Store, output: Output) -> int: + """ + Write the dependency graph as a markdown document with an embedded mermaid diagram. + + args: The parsed arguments. + store: The store to read from. + output: Where to write. + + Returns the process exit code. + """ + + ticketId, key, status = resolveGraphScope(args.scope, args.id, args.key, args.status) + + requireScopeKey(store, key) + + # The flag overrides the configured ceiling for one run, which is what lets a repository render a bigger diagram once without editing its configuration. + maxNodes: int = store.config.maxRoadmapNodes if args.maxNodes is None else args.maxNodes + + roadmap: Roadmap = buildRoadmap(store, ticketId=ticketId, key=key, status=status, maxNodes=maxNodes) - # A configuration is what lets the brief name real keys and real numbering, but its absence is a state the document handles rather than an error, since a person may be anywhere when they go to fetch it. - store: Optional[Store] = Store(config) if config is not None else None + # The document says nothing about what the ceiling dropped, so the person who ran the command is told here instead. Otherwise a diagram that quietly stopped showing its history gives no hint of why. + note: str = f" ({roadmap.dropped} completed ticket(s) omitted)" if roadmap.dropped else "" - return emitDocument(renderHandoff(store), args.output, OUTPUT_ARGUMENT, output) + return emitDocument(roadmap.document, args.output, OUTPUT_ARGUMENT, output, documentPath(store.config, ROADMAP_FILENAME), args.toPrint, note) def commandDeploy(args: argparse.Namespace, output: Output) -> int: diff --git a/src/docket/cli/grammar.py b/src/docket/cli/grammar.py index 5d298a9..9c5346f 100644 --- a/src/docket/cli/grammar.py +++ b/src/docket/cli/grammar.py @@ -16,7 +16,9 @@ from docket import __version__ from docket.core.config import Config, discoverConfig from docket.core.errors import ConflictingArgumentsError, DocketError, InvalidArgumentError, InvalidIdError +from docket.core.handoff import HANDOFF_FILENAME from docket.core.ids import isValidId, isValidKey +from docket.core.roadmap import ROADMAP_FILENAME from docket.core.ticket import STATUSES # MARK: Constants @@ -260,11 +262,7 @@ def buildParser(config: Optional[Config] = None) -> argparse.ArgumentParser: listParser.add_argument("-r", "--ready", action="store_true", help="Keep only tickets whose dependencies are all done. A done ticket is never ready, so this never shows one.") graphParser: argparse.ArgumentParser = commands.add_parser("graph", help="Render the dependency graph as mermaid source.", formatter_class=RichHelpFormatter) - graphParser.add_argument("scope", nargs="?", metavar="SCOPE", help=f"What to scope to, read from its own shape: a ticket id, a key, or a status ({', '.join(STATUSES)}). The flags below are the same three, named explicitly.") - graphScope = graphParser.add_mutually_exclusive_group() - graphScope.add_argument("-i", "--id", help="Scope to one ticket's ancestors and descendants.") - graphScope.add_argument("-k", "--key", help=f"Scope to one key, plus its immediate cross-key neighbors. {keyOptions}") - graphScope.add_argument("-s", "--status", choices=STATUSES, help="Scope to the tickets with this status alone. Nothing outside it is borrowed, so an edge survives only when both of its ends carry the status.") + addScopeArguments(graphParser, keyOptions) graphParser.add_argument("-o", "--output", help="Write to a file rather than to stdout.") keyParser: argparse.ArgumentParser = commands.add_parser("key", help="Inspect and manage the key registry.", formatter_class=RichHelpFormatter) @@ -280,21 +278,37 @@ def buildParser(config: Optional[Config] = None) -> argparse.ArgumentParser: keyRemoveParser: argparse.ArgumentParser = keyCommands.add_parser("remove", help="Remove a key no ticket uses.", formatter_class=RichHelpFormatter) keyRemoveParser.add_argument("key", help=f"The key to remove. {keyOptions}") - # Documents docket ships, rendered against this repository. This is a group rather than a bare command because what it prints is read somewhere else, and more than one such document is plausible. - docsParser: argparse.ArgumentParser = commands.add_parser("docs", help="Print a document docket ships, rendered for this repository.", formatter_class=RichHelpFormatter) + # Documents docket ships, rendered against this repository. This is a group rather than a bare command because each one is read somewhere else, and more than one such document was always plausible. + docsParser: argparse.ArgumentParser = commands.add_parser("docs", help="Write a document docket ships, rendered for this repository.", formatter_class=RichHelpFormatter) docsCommands = docsParser.add_subparsers(dest="docsCommand", metavar="SUBCOMMAND") - # Every document is written the same two ways, so the destination is declared once here and inherited by each one rather than repeated per document. + # Every document reaches its reader the same way, so the destination is declared once here and inherited by each one rather than repeated per document. A document is a file by default, unlike `graph`, because it is written to be kept and read later rather than piped into something else. docsOutput: argparse.ArgumentParser = argparse.ArgumentParser(add_help=False) - docsOutput.add_argument("-o", "--output", help="Write to a file rather than to stdout.") + docsOutput.add_argument("-o", "--output", help="Write to this file instead of the one the document is named for.") + docsOutput.add_argument("-p", "--print", dest="toPrint", action="store_true", help="Print to stdout instead of writing the file. Combined with -o/--output it does both, writing the file and printing it.") docsCommands.add_parser( "handoff", - help="Print the brief that teaches a chat system with no access to this repository how to write tickets for it by hand.", + help=f"Write the brief that teaches a chat system with no access to this repository how to write tickets for it by hand. Lands in {HANDOFF_FILENAME}.", parents=[docsOutput], formatter_class=RichHelpFormatter, ) + roadmapParser: argparse.ArgumentParser = docsCommands.add_parser( + "roadmap", + help=f"Write the dependency graph as a markdown document with an embedded mermaid diagram, for committing alongside the tickets. Lands in {ROADMAP_FILENAME}.", + parents=[docsOutput], + formatter_class=RichHelpFormatter, + ) + addScopeArguments(roadmapParser, keyOptions) + roadmapParser.add_argument( + "-m", + "--max-nodes", + type=int, + dest="maxNodes", + help="How many nodes to aim for, overriding the configured maxRoadmapNodes. Past it the completed tickets furthest from the work still open are dropped, and open tickets are never dropped. Pass 0 for no ceiling.", + ) + commands.add_parser("validate", help="Run every integrity rule.", formatter_class=RichHelpFormatter) deployParser: argparse.ArgumentParser = commands.add_parser("deploy", help="Install docket into a repository.", formatter_class=RichHelpFormatter) @@ -306,6 +320,24 @@ def buildParser(config: Optional[Config] = None) -> argparse.ArgumentParser: return parser +def addScopeArguments(parser: argparse.ArgumentParser, keyOptions: str) -> None: + """ + Add the three ways of scoping a graph to a parser, in both their bare and their explicit spelling. + + Both commands that draw a graph offer the same scopes, read by the same rules, and resolved by the same `resolveGraphScope`. Declaring them once is what keeps the two from drifting into disagreeing about what a bare token means. + + parser: The parser to add the arguments to. + keyOptions: The sentence describing the registered keys, already built. + """ + + parser.add_argument("scope", nargs="?", metavar="SCOPE", help=f"What to scope to, read from its own shape: a ticket id, a key, or a status ({', '.join(STATUSES)}). The flags below are the same three, named explicitly.") + + scopeGroup = parser.add_mutually_exclusive_group() + scopeGroup.add_argument("-i", "--id", help="Scope to one ticket's ancestors and descendants.") + scopeGroup.add_argument("-k", "--key", help=f"Scope to one key, plus its immediate cross-key neighbors. {keyOptions}") + scopeGroup.add_argument("-s", "--status", choices=STATUSES, help="Scope to the tickets with this status alone. Nothing outside it is borrowed, so an edge survives only when both of its ends carry the status.") + + def buildTicketParser(commands: argparse._SubParsersAction, priorityOptions: str) -> argparse.ArgumentParser: """ Build the branch that works with one ticket, named by its id. diff --git a/src/docket/core/config.py b/src/docket/core/config.py index 5e7ffa7..ecc9cb1 100644 --- a/src/docket/core/config.py +++ b/src/docket/core/config.py @@ -36,6 +36,9 @@ DEFAULT_MAX_PRIORITY: int = 4 DEFAULT_LOCK_TIMEOUT: float = 5.0 +# How many nodes a rendered roadmap aims to hold. Mermaid lays out a few hundred before a diagram stops being readable at any zoom, and a repository with more history than that would rather lose the history than the shape of what is ahead. +DEFAULT_MAX_ROADMAP_NODES: int = 200 + # MARK: Classes @@ -69,6 +72,7 @@ def __init__(self, path: Path, document: TOMLDocument) -> None: self.defaultPriority: int = readInt(document, "defaultPriority", ConfigError, source, DEFAULT_PRIORITY) self.maxPriority: int = readInt(document, "maxPriority", ConfigError, source, DEFAULT_MAX_PRIORITY) self.lockTimeout: float = readFloat(document, "lockTimeout", ConfigError, source, DEFAULT_LOCK_TIMEOUT) + self.maxRoadmapNodes: int = readInt(document, "maxRoadmapNodes", ConfigError, source, DEFAULT_MAX_ROADMAP_NODES) # A default outside the allowed band would make every created ticket invalid, so catch it at load. if not 0 <= self.defaultPriority <= self.maxPriority: @@ -78,6 +82,10 @@ def __init__(self, path: Path, document: TOMLDocument) -> None: if self.lockTimeout <= 0: raise ConfigError(f"lockTimeout {self.lockTimeout} in {self.path} must be greater than 0. It is a wait in seconds.") + # Zero is the documented way to ask for no ceiling at all, so only a negative number is meaningless here. + if self.maxRoadmapNodes < 0: + raise ConfigError(f"maxRoadmapNodes {self.maxRoadmapNodes} in {self.path} cannot be negative. It is a node count, and 0 means no ceiling.") + # MARK: Properties @property diff --git a/src/docket/core/graph.py b/src/docket/core/graph.py index 7f5e9a3..e0703cc 100644 --- a/src/docket/core/graph.py +++ b/src/docket/core/graph.py @@ -13,7 +13,7 @@ from docket.core.ids import parseId from docket.core.store import TicketSet -from docket.core.ticket import Ticket +from docket.core.ticket import STATUS_DONE, Ticket # MARK: Classes @@ -136,6 +136,22 @@ def nodesForKey(self, key: str) -> list[GraphNode]: return sorted((node for node in self.nodes.values() if node.key == key), key=lambda node: parseId(node.id)[1]) +@dataclass(frozen=True) +class CulledGraph: + """ + A graph narrowed to fit a node ceiling, and how much of it was left behind. + + The count travels beside the graph rather than inside it, because a renderer has no use for it and the caller that asked for the cull does. A document meant to show where the work is going should not spend a line apologizing for the history it dropped, while the person who ran the command is owed the fact that something was dropped at all. + """ + + # MARK: Properties + + graph: ResolvedGraph + + # How many nodes the ceiling cost, which is zero whenever the graph already fit. + dropped: int = 0 + + # MARK: Functions @@ -248,6 +264,84 @@ def subgraphForStatus(graph: ResolvedGraph, status: str) -> ResolvedGraph: return _restrict(graph, members, scope=status) +def scopeGraph(graph: ResolvedGraph, ticketId: Optional[str] = None, key: Optional[str] = None, status: Optional[str] = None) -> ResolvedGraph: + """ + Apply whichever of the three scopes was asked for, and return the graph untouched when none was. + + The three remain exclusive, so the first one set is the one that applies and a caller passing two has already been refused by the grammar that read them. Checking that a key is registered or that a status is spelled correctly belongs to the caller, since the CLI and the server learn those from different places and report them differently. + + graph: The graph to scope. + ticketId: The ticket to center on, or `None`. + key: The key to scope to, or `None`. + status: The status to scope to, or `None`. + + Returns the scoped graph, or the same graph when nothing scoped it. + """ + + if ticketId is not None: + return subgraphForId(graph, ticketId) + + if key is not None: + return subgraphForKey(graph, key) + + if status is not None: + return subgraphForStatus(graph, status) + + return graph + + +def cullGraph(graph: ResolvedGraph, maxNodes: int) -> CulledGraph: + """ + Narrow a graph toward a node ceiling by dropping the finished work furthest from the work still open. + + Every unfinished ticket survives, whatever the ceiling says, and the finished ones are then admitted ring by ring outward from that seed along both edge directions. So a completed ticket something open still depends on stays, its own dependencies stay behind it while there is room, and the history nothing open reaches any more is what goes first. + + The ceiling is therefore a target rather than a cap. A repository whose open work alone exceeds it keeps all of that work and every finished ticket is dropped, because a roadmap that hides a live ticket is worse than one that renders slowly, and choosing which live ticket to hide is not a choice this can make well. + + A ring that will not fit whole is filled in id order, so the same graph always culls to the same nodes and a committed document does not churn between runs. + + graph: The graph to narrow. + maxNodes: The node count to aim for, or zero and below for no ceiling at all. + + Returns the narrowed graph and the number of nodes it cost. + """ + + # Nothing to do when no ceiling was asked for, or when the graph already sits under the one that was. + if maxNodes <= 0 or len(graph) <= maxNodes: + return CulledGraph(graph=graph) + + # Seed with everything still open, which is the part of the graph a reader is looking for and the part that is never given up. + included: set[str] = {node.id for node in graph.nodes.values() if node.status != STATUS_DONE} + + # Walk outward from that seed, one distance ring at a time, until the ceiling is met or the reachable graph runs out. + frontier: set[str] = included + while len(included) < maxNodes: + ring: set[str] = set() + for nodeId in frontier: + node: GraphNode = graph.nodes[nodeId] + ring |= set(node.requires) | set(node.requiredBy) + + # A node keeps its full edge lists through a scoping, so a neighbor may name something this graph does not hold. + ring &= graph.nodes.keys() + ring -= included + + # Nothing further out is reachable, so the ceiling is met with room to spare. + if not ring: + break + + # A ring that overflows is taken in part, by id, since every member of it sits at the same distance and nothing else distinguishes them. + remaining: int = maxNodes - len(included) + if len(ring) > remaining: + included |= set(_orderedIds(ring)[:remaining]) + break + + included |= ring + frontier = ring + + # The scope is carried across, since narrowing a graph does not change what it was scoped to. + return CulledGraph(graph=_restrict(graph, included, scope=graph.scope), dropped=len(graph) - len(included)) + + def dependencyContext(ticketSet: TicketSet, ticketId: str) -> dict[str, list[dict[str, object]]]: """ Resolve the context a raw ticket file deliberately does not carry. diff --git a/src/docket/core/handoff.py b/src/docket/core/handoff.py index c50d350..42c7ebd 100644 --- a/src/docket/core/handoff.py +++ b/src/docket/core/handoff.py @@ -11,21 +11,19 @@ from dataclasses import dataclass from typing import Any, Optional -from jinja2 import Environment, StrictUndefined, Template - from docket.core.config import DEFAULT_MAX_PRIORITY, DEFAULT_PRIORITY, DEFAULT_ROOT, DEFAULT_TODO_DIR, Config from docket.core.ids import nextId -from docket.core.resources import readPackageText from docket.core.store import Store +from docket.core.templating import renderDocument # MARK: Constants -# The directory inside the package holding documents written to be read by someone, kept apart from `templates` because those are files a repository receives rather than text a person is handed. -DOCS_DIRECTORY: str = "docs" - # The brief itself. HANDOFF_TEMPLATE: str = "writingTicketsOffsite.md.jinja" +# What the brief is called once it has been written into a repository. +HANDOFF_FILENAME: str = "handoff.md" + # MARK: Classes @@ -47,18 +45,6 @@ class KeyBriefing: # MARK: Functions -def readDocument(name: str) -> str: - """ - Read a document shipped inside the package. - - name: The document filename. - - Returns the document text. - """ - - return readPackageText(DOCS_DIRECTORY, name) - - def buildKeyBriefings(store: Store) -> list[KeyBriefing]: """ Describe every registered key, with the next id free under it. @@ -117,18 +103,4 @@ def renderHandoff(store: Optional[Store] = None) -> str: Returns the rendered document. """ - template: Template = _buildEnvironment().from_string(readDocument(HANDOFF_TEMPLATE)) - - return template.render(buildContext(store)) - - -def _buildEnvironment() -> Environment: - """ - Build the environment every shipped document renders through. - - Autoescaping is off because the output is markdown a person reads, and escaping it would corrupt the very syntax the brief is teaching. An undefined name raises rather than rendering as nothing, so a template naming something the context does not carry fails here instead of reaching the reader as a hole in a sentence. - - Returns the environment. - """ - - return Environment(undefined=StrictUndefined, trim_blocks=True, lstrip_blocks=True, keep_trailing_newline=True, autoescape=False) + return renderDocument(HANDOFF_TEMPLATE, buildContext(store)) diff --git a/src/docket/core/roadmap.py b/src/docket/core/roadmap.py new file mode 100644 index 0000000..c1b65f8 --- /dev/null +++ b/src/docket/core/roadmap.py @@ -0,0 +1,87 @@ +""" +Docket Roadmap + +Rendering the committable roadmap document. + +The document is one markdown file a repository keeps under version control, so what it holds is a diagram and the key to reading it, and nothing that would change between two runs over the same tickets. +""" + +# MARK: Imports + +from dataclasses import dataclass +from typing import Any, Optional + +from docket.core.graph import CulledGraph, ResolvedGraph, cullGraph, resolveGraph, scopeGraph +from docket.core.mermaid import renderGraph +from docket.core.store import Store +from docket.core.templating import renderDocument + +# MARK: Constants + +# The document itself. +ROADMAP_TEMPLATE: str = "roadmap.md.jinja" + +# What the roadmap is called once it has been written into a repository. +ROADMAP_FILENAME: str = "roadmap.md" + +# The heading an unscoped roadmap carries. +ROADMAP_TITLE: str = "Roadmap" + +# MARK: Classes + + +@dataclass(frozen=True) +class Roadmap: + """ + A rendered roadmap, and what the node ceiling cost to produce it. + + The count rides along because the document deliberately does not mention it. A roadmap is about where the work is going rather than what has already been finished, so the omission is worth telling the person who ran the command and not worth spending a line of the file on. + """ + + # MARK: Properties + + document: str + + # How many nodes the ceiling dropped, which is zero whenever the whole graph fit. + dropped: int = 0 + + +# MARK: Functions + + +def buildContext(graph: ResolvedGraph) -> dict[str, Any]: + """ + Gather what the document names about the graph it is drawn from. + + graph: The graph the document draws, already scoped and culled. + + Returns the render context. + """ + + return { + # The scope is whatever narrowed the graph, so a scoped roadmap says so in its own heading rather than looking like the whole project. + "title": f"{ROADMAP_TITLE}: {graph.scope}" if graph.scope is not None else ROADMAP_TITLE, + "mermaid": renderGraph(graph), + # The legend explains a dashed border only where one appears, since a key is the one scope that borrows from outside itself. + "hasExternal": any(node.isExternal for node in graph.nodes.values()), + } + + +def buildRoadmap(store: Store, ticketId: Optional[str] = None, key: Optional[str] = None, status: Optional[str] = None, maxNodes: int = 0) -> Roadmap: + """ + Render the roadmap for a repository. + + Scoping happens before culling, so the ceiling is measured against what will actually be drawn rather than against the whole repository. + + store: The store to read the tickets from. + ticketId: The ticket to center on, or `None`. + key: The key to scope to, or `None`. + status: The status to scope to, or `None`. + maxNodes: The node count to aim for, or zero for no ceiling. + + Returns the rendered document and what the ceiling cost. + """ + + culled: CulledGraph = cullGraph(scopeGraph(resolveGraph(store.loadAll()), ticketId, key, status), maxNodes) + + return Roadmap(document=renderDocument(ROADMAP_TEMPLATE, buildContext(culled.graph)), dropped=culled.dropped) diff --git a/src/docket/core/templating.py b/src/docket/core/templating.py new file mode 100644 index 0000000..818293a --- /dev/null +++ b/src/docket/core/templating.py @@ -0,0 +1,61 @@ +""" +Docket Templating + +The environment every document docket renders is built through. + +This holds the settings and nothing else. What each document says belongs to the module that renders it, so a second document costs another renderer rather than another environment. +""" + +# MARK: Imports + +from jinja2 import Environment, StrictUndefined, Template + +from docket.core.resources import readPackageText + +# MARK: Constants + +# The directory inside the package holding documents written to be read by someone, kept apart from `templates` because those are files a repository receives rather than text a person is handed. +DOCS_DIRECTORY: str = "docs" + +# MARK: Functions + + +def buildEnvironment() -> Environment: + """ + Build the environment every shipped document renders through. + + Autoescaping is off because the output is markdown a person reads, and escaping it would corrupt the very syntax a brief may be teaching. An undefined name raises rather than rendering as nothing, so a template naming something the context does not carry fails here instead of reaching the reader as a hole in a sentence. + + Returns the environment. + """ + + return Environment(undefined=StrictUndefined, trim_blocks=True, lstrip_blocks=True, keep_trailing_newline=True, autoescape=False) + + +def readDocument(name: str) -> str: + """ + Read a document shipped inside the package. + + name: The document filename. + + Returns the document text. + """ + + return readPackageText(DOCS_DIRECTORY, name) + + +def renderDocument(name: str, context: dict[str, object]) -> str: + """ + Render one shipped document against a context. + + Reading and rendering are one step here because no caller has ever wanted one without the other, and keeping them together is what leaves each document's module holding only its own context. + + name: The document filename. + context: The names the template renders against. + + Returns the rendered document. + """ + + template: Template = buildEnvironment().from_string(readDocument(name)) + + return template.render(context) diff --git a/src/docket/docs/roadmap.md.jinja b/src/docket/docs/roadmap.md.jinja new file mode 100644 index 0000000..1f34645 --- /dev/null +++ b/src/docket/docs/roadmap.md.jinja @@ -0,0 +1,22 @@ +# {{ title }} + +> Generated with `docket docs roadmap`. + +```mermaid +{{ mermaid }}``` + +## Legend + +| Shape | Status | +|---|---| +| `[ ]` | todo, not started | +| `{ }` | wip, in flight | +| `( )` | done | + +Arrows indicate required order of operations. + +Each node's border represents its priority. Heaviest is higher priority. +{% if hasExternal %} + +A dashed border indicates a ticket from outside this roadmap's scope that is shown because something inside the scope depends on it or is depended on by it. +{% endif %} diff --git a/src/docket/server.py b/src/docket/server.py index 8330dec..af9ffb0 100644 --- a/src/docket/server.py +++ b/src/docket/server.py @@ -32,7 +32,7 @@ from docket import __version__ from docket.core.config import Config, discoverConfig -from docket.core.graph import Readiness, ResolvedGraph, dependencyContext, resolveGraph, subgraphForId, subgraphForKey, subgraphForStatus, ticketReadiness +from docket.core.graph import Readiness, ResolvedGraph, dependencyContext, resolveGraph, scopeGraph, ticketReadiness from docket.core.mermaid import renderGraph from docket.core.store import Store, TicketResult, TicketSet from docket.core.ticket import Ticket, requireKnownStatus @@ -235,16 +235,9 @@ async def graphTool(id: Optional[str] = None, key: Optional[str] = None, status: """ store: Store = _store() - graph: ResolvedGraph = resolveGraph(store.loadAll()) - - # Scope when asked, narrowest request first. - if id is not None: - graph = subgraphForId(graph, id) - elif key is not None: - graph = subgraphForKey(graph, key) - elif status is not None: - # The CLI has `choices` to reject a status nobody uses, and without the same check here an unreadable one would render an empty graph that reads as an answer. - graph = subgraphForStatus(graph, requireKnownStatus(status)) + + # The CLI has `choices` to reject a status nobody uses, and without the same check here an unreadable one would render an empty graph that reads as an answer. + graph: ResolvedGraph = scopeGraph(resolveGraph(store.loadAll()), id, key, None if status is None else requireKnownStatus(status)) return _json({"scope": graph.scope, "nodeCount": len(graph), "mermaid": renderGraph(graph)}) diff --git a/src/docket/templates/CLAUDE.md b/src/docket/templates/CLAUDE.md index 37ae41a..b00a7d1 100644 --- a/src/docket/templates/CLAUDE.md +++ b/src/docket/templates/CLAUDE.md @@ -55,7 +55,7 @@ Filenames are frozen at creation. Retitling a ticket deliberately does not renam ## Tickets written outside this repository -Docket ships a second document, printed by `docket docs handoff`, written for a chat system that has no access to this repository and has to write ticket files by hand. +Docket ships a second document, written by `docket docs handoff`, for a chat system that has no access to this repository and has to write ticket files by hand. Its rules are deliberately the opposite of the ones above, because its reader has no tools to call. Do not follow it here. A file produced that way is an ordinary ticket the moment it lands, so `validate` is what confirms it and the tools above are what change it afterwards. @@ -80,6 +80,12 @@ A word carrying an uppercase letter past its first character, or a digit anywher | Add a new key | `add_key`, after asking the user | | Check the set is sound | `validate` | +## The committed roadmap + +This repository may keep a `roadmap.md`, which is the dependency graph as a diagram a reader can open on the repository page. It is generated, so never edit it by hand. Regenerate it with `docket docs roadmap` from a terminal, which is a command rather than a tool because it writes a file outside the ticket directories. + +The `graph` tool is what you call to see the same graph for yourself. It is the roadmap without the wrapper, and calling it changes nothing. + ## Dependencies point one way A ticket declares what it `requires`. It never declares what it blocks. diff --git a/src/docket/templates/docket.toml b/src/docket/templates/docket.toml index 1e257b8..d1d9363 100644 --- a/src/docket/templates/docket.toml +++ b/src/docket/templates/docket.toml @@ -12,6 +12,10 @@ doneDir = "done" defaultPriority = 2 maxPriority = 4 +# How many nodes `docket docs roadmap` aims to draw. Past this it drops the completed tickets furthest from the work still open. +# Set it to 0 for no ceiling, and remember that mermaid stops being readable well before a thousand nodes. +maxRoadmapNodes = 200 + # How long, in seconds, one docket process waits for another to finish writing before giving up. # Writes are serialized across processes, so a CLI command and a running MCP server never overwrite each other. lockTimeout = 5.0 diff --git a/tests/test_cli.py b/tests/test_cli.py index b32fa4c..570b49c 100644 --- a/tests/test_cli.py +++ b/tests/test_cli.py @@ -33,6 +33,8 @@ ) from docket.core.config import Config, loadConfig from docket.core.errors import ConflictingArgumentsError, InvalidArgumentError, InvalidIdError +from docket.core.handoff import HANDOFF_FILENAME +from docket.core.roadmap import ROADMAP_FILENAME # MARK: Fixtures @@ -788,21 +790,57 @@ def testTheOldFlatCommandsAreGone(inRepo: Path, command: list[str]) -> None: assert excInfo.value.code == EXIT_USAGE -def testDocsHandoffWritesTheBriefToStdout(inRepo: Path, capsys: pytest.CaptureFixture[str]) -> None: +def testDocsHandoffWritesItsPrescribedFile(inRepo: Path, capsys: pytest.CaptureFixture[str]) -> None: """ - The brief is meant to be redirected to a file or pasted into another chat, so it bypasses `rich` exactly as mermaid source does. + A document is written to be kept and read later rather than piped, so it lands in the repository without being asked to. """ assert main(["docs", "handoff"]) == EXIT_OK + assert "Wrote" in capsys.readouterr().out + + written: str = (inRepo / HANDOFF_FILENAME).read_text(encoding="utf-8") + + assert written.startswith("# Writing Tickets for Docket, Offsite\n") + + # Rendered inside a repository, the brief names that repository's registry rather than teaching the reader to invent one. + assert "`CORE-1`" in written + assert "No keys were available" not in written + + +def testDocsHandoffPrintsTheBriefToStdout(inRepo: Path, capsys: pytest.CaptureFixture[str]) -> None: + """ + Printing is what pastes the brief into another chat, so it bypasses `rich` exactly as mermaid source does and writes no file on the way. + """ + + assert main(["docs", "handoff", "--print"]) == EXIT_OK out: str = capsys.readouterr().out assert out.startswith("# Writing Tickets for Docket, Offsite\n") assert "\x1b" not in out + assert "Wrote" not in out - # Rendered inside a repository, the brief names that repository's registry rather than teaching the reader to invent one. - assert "`CORE-1`" in out - assert "No keys were available" not in out + # Printing replaces the prescribed file rather than adding to it, which is what keeps a bare print clean enough to pipe. + assert not (inRepo / HANDOFF_FILENAME).exists() + + +def testDocsPrintAndOutputTogetherDoBoth(inRepo: Path, capsys: pytest.CaptureFixture[str]) -> None: + """ + The two are deliberately not exclusive, so a caller can keep the file and read it in the same breath. + """ + + target: Path = inRepo / "brief.md" + + assert main(["docs", "handoff", "--print", "--output", str(target)]) == EXIT_OK + + out: str = capsys.readouterr().out + + assert "Wrote" in out + assert "# Writing Tickets for Docket, Offsite" in out + assert target.is_file() + + # The named destination replaced the prescribed one rather than joining it. + assert not (inRepo / HANDOFF_FILENAME).exists() def testDocsHandoffRendersOutsideARepository(tmp_path: Path, capsys: pytest.CaptureFixture[str]) -> None: @@ -813,13 +851,28 @@ def testDocsHandoffRendersOutsideARepository(tmp_path: Path, capsys: pytest.Capt previous: str = os.getcwd() os.chdir(tmp_path) try: - assert main(["docs", "handoff"]) == EXIT_OK + assert main(["docs", "handoff", "--print"]) == EXIT_OK finally: os.chdir(previous) assert "No keys were available" in capsys.readouterr().out +def testDocsHandoffWritesBesideAMissingConfiguration(tmp_path: Path) -> None: + """ + Without a repository there is no root to write into, so the working directory is the only honest place the prescribed file can land. + """ + + previous: str = os.getcwd() + os.chdir(tmp_path) + try: + assert main(["docs", "handoff"]) == EXIT_OK + finally: + os.chdir(previous) + + assert (tmp_path / HANDOFF_FILENAME).is_file() + + def testDocsHandoffWritesToAnOutputPath(inRepo: Path, capsys: pytest.CaptureFixture[str]) -> None: """ The brief is carried somewhere else, so writing it straight to a file saves the redirect a person would otherwise have to type. @@ -830,6 +883,9 @@ def testDocsHandoffWritesToAnOutputPath(inRepo: Path, capsys: pytest.CaptureFixt assert main(["docs", "handoff", "--output", str(target)]) == EXIT_OK assert "Wrote" in capsys.readouterr().out + # A named destination replaces the prescribed one rather than adding to it. + assert not (inRepo / HANDOFF_FILENAME).exists() + # The file holds the document itself, rendered for this repository rather than the fallback. written: str = target.read_text(encoding="utf-8") @@ -855,7 +911,126 @@ def testDocsRefusesAnUnknownSubcommand(inRepo: Path, capsys: pytest.CaptureFixtu """ assert main(["docs"]) == EXIT_USAGE - assert "Expected one of: handoff." in capsys.readouterr().err + assert "Expected one of: handoff, roadmap." in capsys.readouterr().err + + +def testDocsRoadmapWritesItsPrescribedFile(inRepo: Path, capsys: pytest.CaptureFixture[str]) -> None: + """ + The roadmap exists to be committed alongside the tickets, so the bare command is the whole of what a person or a hook has to run. + """ + + main(["new", "CORE", "App Shell"]) + capsys.readouterr() + + assert main(["docs", "roadmap"]) == EXIT_OK + assert "Wrote" in capsys.readouterr().out + + written: str = (inRepo / ROADMAP_FILENAME).read_text(encoding="utf-8") + + assert written.startswith("# Roadmap\n") + + # The fence is the whole difference between this and what `graph` prints. + assert "```mermaid\n" in written + assert "CORE-1" in written + + +def testDocsRoadmapPrintsWithoutWriting(inRepo: Path, capsys: pytest.CaptureFixture[str]) -> None: + """ + Printing is how the document is read without leaving anything behind, which matters most for a file the repository would otherwise commit. + """ + + main(["new", "CORE", "App Shell"]) + capsys.readouterr() + + assert main(["docs", "roadmap", "--print"]) == EXIT_OK + + out: str = capsys.readouterr().out + + assert out.startswith("# Roadmap\n") + assert "\x1b" not in out + assert not (inRepo / ROADMAP_FILENAME).exists() + + +def testDocsRoadmapScopesFromABareToken(inRepo: Path, capsys: pytest.CaptureFixture[str]) -> None: + """ + The roadmap reads a scope by the same rules the graph does, since both resolve it through the same grammar. + """ + + main(["new", "CORE", "App Shell"]) + main(["new", "GEN", "Map Generation"]) + capsys.readouterr() + + assert main(["docs", "roadmap", "GEN", "--print"]) == EXIT_OK + + out: str = capsys.readouterr().out + + assert out.startswith("# Roadmap: GEN\n") + assert "GEN-1" in out + + # A key scope borrows only what neighbors it, and nothing here does. + assert "CORE-1" not in out + + +def testDocsRoadmapRefusesAnUnregisteredKey(inRepo: Path, capsys: pytest.CaptureFixture[str]) -> None: + """ + Scoping to a key nobody registered would draw an empty diagram, which reads as an answer rather than the typo it is. + """ + + assert main(["docs", "roadmap", "NOPE", "--print"]) == EXIT_USAGE + assert "is not registered" in capsys.readouterr().err + + +def testDocsRoadmapReportsWhatTheCeilingDropped(inRepo: Path, capsys: pytest.CaptureFixture[str]) -> None: + """ + The document says nothing about the omission, so the confirmation line is the only place a person learns the diagram is not the whole graph. + """ + + main(["new", "CORE", "App Shell"]) + main(["new", "CORE", "Second"]) + main(["CORE-1", "done"]) + capsys.readouterr() + + assert main(["docs", "roadmap", "--max-nodes", "1"]) == EXIT_OK + + out: str = capsys.readouterr().out + + assert "1 completed ticket(s) omitted" in out + + # Open work survives a ceiling it does not fit under, and the finished ticket is what paid for it. + written: str = (inRepo / ROADMAP_FILENAME).read_text(encoding="utf-8") + + assert "CORE-2" in written + assert "CORE-1" not in written + + +def testDocsRoadmapSaysNothingWhenNothingWasDropped(inRepo: Path, capsys: pytest.CaptureFixture[str]) -> None: + """ + A repository under the ceiling never hears about the ceiling, since a note about nothing is noise. + """ + + main(["new", "CORE", "App Shell"]) + capsys.readouterr() + + assert main(["docs", "roadmap"]) == EXIT_OK + assert "omitted" not in capsys.readouterr().out + + +def testDocsRoadmapCeilingComesFromTheConfiguration(inRepo: Path, capsys: pytest.CaptureFixture[str]) -> None: + """ + The ceiling is a property of how a repository renders its roadmap, so the bare command honors it without the flag. + """ + + main(["new", "CORE", "App Shell"]) + main(["new", "CORE", "Second"]) + main(["CORE-1", "done"]) + + # The field goes above `[keys]`, since anything after that header would be read as a key rather than as a top-level setting. + configPath: Path = inRepo / ".docket.toml" + configPath.write_text(configPath.read_text(encoding="utf-8").replace("[keys]", "maxRoadmapNodes = 1\n\n[keys]"), encoding="utf-8", newline="\n") + capsys.readouterr() + + assert main(["docs", "roadmap"]) == EXIT_OK + assert "1 completed ticket(s) omitted" in capsys.readouterr().out def testGraphWritesBareMermaidToStdout(inRepo: Path, capsys: pytest.CaptureFixture[str]) -> None: diff --git a/tests/test_config.py b/tests/test_config.py index 4fe8d3f..a5d87a5 100644 --- a/tests/test_config.py +++ b/tests/test_config.py @@ -10,7 +10,7 @@ import pytest -from docket.core.config import DEFAULT_LOCK_TIMEOUT, Config, discoverConfig, findConfigPath, loadConfig +from docket.core.config import DEFAULT_LOCK_TIMEOUT, DEFAULT_MAX_ROADMAP_NODES, Config, discoverConfig, findConfigPath, loadConfig from docket.core.errors import ConfigError, ConfigNotFoundError, InvalidKeyError, UnknownKeyError # MARK: Functions @@ -155,6 +155,40 @@ def testALockTimeoutOfZeroOrLessIsRejected(tmp_path: Path, timeout: str) -> None loadConfig(configPath) +def testAnAbsentRoadmapCeilingFallsBackToTheDefault(tmp_path: Path) -> None: + """ + An upgrade never rewrites a configuration, so every repository deployed before the roadmap existed carries no ceiling and must keep loading. + """ + + configPath: Path = tmp_path / ".docket.toml" + configPath.write_text('root = "docs/tickets"\n', encoding="utf-8", newline="\n") + + assert loadConfig(configPath).maxRoadmapNodes == DEFAULT_MAX_ROADMAP_NODES + + +def testARoadmapCeilingOfZeroIsAccepted(tmp_path: Path) -> None: + """ + Zero is the documented way to ask for no ceiling at all, so it is a setting rather than a mistake. + """ + + configPath: Path = tmp_path / ".docket.toml" + configPath.write_text("maxRoadmapNodes = 0\n", encoding="utf-8", newline="\n") + + assert loadConfig(configPath).maxRoadmapNodes == 0 + + +def testANegativeRoadmapCeilingIsRejected(tmp_path: Path) -> None: + """ + A node count below zero names nothing, and zero already means what a writer reaching for it would have meant. + """ + + configPath: Path = tmp_path / ".docket.toml" + configPath.write_text("maxRoadmapNodes = -1\n", encoding="utf-8", newline="\n") + + with pytest.raises(ConfigError): + loadConfig(configPath) + + def testRegisteredKeysAreReadWithTheirDescriptions(config: Config) -> None: """ Every scalar entry under `[keys]` is a key, and the comments between them are not. diff --git a/tests/test_graph.py b/tests/test_graph.py index 38a3d00..e18eb53 100644 --- a/tests/test_graph.py +++ b/tests/test_graph.py @@ -1,14 +1,16 @@ """ Graph Tests -Cover reverse edge derivation, scoped traversal, dependency context, readiness, and cycle detection. +Cover reverse edge derivation, scoped traversal, culling, dependency context, readiness, and cycle detection. """ # MARK: Imports +from typing import Optional + import pytest -from docket.core.graph import Edge, Readiness, ResolvedGraph, dependencyContext, findCycles, readyTickets, resolveGraph, subgraphForId, subgraphForKey, subgraphForStatus, ticketReadiness +from docket.core.graph import CulledGraph, Edge, Readiness, ResolvedGraph, cullGraph, dependencyContext, findCycles, readyTickets, resolveGraph, scopeGraph, subgraphForId, subgraphForKey, subgraphForStatus, ticketReadiness from docket.core.store import TicketSet from docket.core.ticket import Ticket @@ -421,3 +423,168 @@ def testTraversalSurvivesACycle() -> None: scoped: ResolvedGraph = subgraphForId(graph, "CORE-3") assert sorted(scoped.nodes) == ["CORE-1", "CORE-2", "CORE-3"] + + +def testScopeGraphReturnsTheWholeGraphWhenNothingScopesIt() -> None: + """ + Both commands that draw a graph share one scoping switch, so the unscoped case has to be the identity rather than an empty result. + """ + + graph: ResolvedGraph = resolveGraph(buildSet(("CORE-1", []), ("GEN-1", ["CORE-1"]))) + + assert scopeGraph(graph) is graph + + +@pytest.mark.parametrize( + ("ticketId", "key", "status", "expected"), + [ + ("CORE-1", None, None, ["CORE-1", "GEN-1"]), + (None, "GEN", None, ["CORE-1", "GEN-1"]), + (None, None, "todo", ["CORE-1", "GEN-1"]), + (None, None, "done", ["HEAD-1"]), + ], +) +def testScopeGraphAppliesWhicheverScopeWasNamed(ticketId: Optional[str], key: Optional[str], status: Optional[str], expected: list[str]) -> None: + """ + The three scopes reach the same three traversals through the switch that the callers used to each write out for themselves. + + ticketId: The id scope under test, or `None`. + key: The key scope under test, or `None`. + status: The status scope under test, or `None`. + expected: The ids the scope should keep. + """ + + graph: ResolvedGraph = resolveGraph(buildStatusSet(("CORE-1", "todo", []), ("GEN-1", "todo", ["CORE-1"]), ("HEAD-1", "done", []))) + + assert sorted(scopeGraph(graph, ticketId, key, status).nodes) == expected + + +def testCullingIsSkippedWithoutACeiling() -> None: + """ + Zero is the documented way to ask for no ceiling, so the graph has to come back untouched rather than emptied. + """ + + graph: ResolvedGraph = resolveGraph(buildSet(("CORE-1", []), ("CORE-2", ["CORE-1"]))) + + culled: CulledGraph = cullGraph(graph, 0) + + assert culled.graph is graph + assert culled.dropped == 0 + + +def testCullingIsSkippedWhenTheGraphAlreadyFits() -> None: + """ + A graph under the ceiling is not narrowed, so a repository small enough never pays for the feature at all. + """ + + graph: ResolvedGraph = resolveGraph(buildSet(("CORE-1", []), ("CORE-2", ["CORE-1"]))) + + culled: CulledGraph = cullGraph(graph, 2) + + assert culled.graph is graph + assert culled.dropped == 0 + + +def testCullingDropsTheFinishedWorkFurthestFromOpenWork() -> None: + """ + The finished tickets are admitted ring by ring outward from the open ones, so the history nothing open still reaches is what goes first. + """ + + # A chain of finished work behind one open ticket, which makes distance the only thing telling the finished tickets apart. + graph: ResolvedGraph = resolveGraph( + buildStatusSet( + ("CORE-1", "done", []), + ("CORE-2", "done", ["CORE-1"]), + ("CORE-3", "done", ["CORE-2"]), + ("CORE-4", "todo", ["CORE-3"]), + ) + ) + + culled: CulledGraph = cullGraph(graph, 3) + + assert sorted(culled.graph.nodes) == ["CORE-2", "CORE-3", "CORE-4"] + assert culled.dropped == 1 + + +def testCullingDropsFinishedWorkNothingOpenReaches() -> None: + """ + A finished ticket disconnected from everything still open is never reached by the walk, so it goes however much room is left. + """ + + graph: ResolvedGraph = resolveGraph( + buildStatusSet( + ("CORE-1", "done", []), + ("CORE-2", "todo", ["CORE-1"]), + ("GEN-1", "done", []), + ("GEN-2", "done", ["GEN-1"]), + ) + ) + + culled: CulledGraph = cullGraph(graph, 2) + + assert sorted(culled.graph.nodes) == ["CORE-1", "CORE-2"] + assert culled.dropped == 2 + + +def testCullingNeverDropsOpenWork() -> None: + """ + The ceiling is a target rather than a cap, because a roadmap that hides a live ticket is worse than one that renders slowly. + """ + + graph: ResolvedGraph = resolveGraph(buildStatusSet(*[(f"CORE-{number}", "todo", []) for number in range(1, 6)], ("GEN-1", "done", []))) + + culled: CulledGraph = cullGraph(graph, 2) + + assert sorted(culled.graph.nodes) == ["CORE-1", "CORE-2", "CORE-3", "CORE-4", "CORE-5"] + + # Only the finished ticket could be given up, and giving it up still leaves the graph over the ceiling. + assert culled.dropped == 1 + + +def testCullingFillsAnOverflowingRingByIdOrder() -> None: + """ + Every member of one ring sits at the same distance, so the tie is broken by id to keep a committed document from churning between runs. + """ + + graph: ResolvedGraph = resolveGraph( + buildStatusSet( + ("CORE-10", "done", []), + ("CORE-2", "done", []), + ("CORE-3", "done", []), + ("CORE-4", "todo", ["CORE-2", "CORE-3", "CORE-10"]), + ) + ) + + culled: CulledGraph = cullGraph(graph, 3) + + # `CORE-2` and `CORE-3` are the first two numerically, which is what orders them rather than the order they were written in. + assert sorted(culled.graph.nodes) == ["CORE-2", "CORE-3", "CORE-4"] + assert culled.dropped == 1 + + +def testCullingNarrowsEdgesToWhatSurvived() -> None: + """ + An edge whose other end was dropped cannot be drawn, so the restriction the scopes already perform is what the cull reuses. + """ + + graph: ResolvedGraph = resolveGraph( + buildStatusSet( + ("CORE-1", "done", []), + ("CORE-2", "done", ["CORE-1"]), + ("CORE-3", "todo", ["CORE-2"]), + ) + ) + + culled: CulledGraph = cullGraph(graph, 2) + + assert culled.graph.edges == [Edge(fromId="CORE-2", toId="CORE-3")] + + +def testCullingKeepsTheScope() -> None: + """ + Narrowing a graph does not change what it was scoped to, and the renderer titles the document from that scope. + """ + + graph: ResolvedGraph = subgraphForKey(resolveGraph(buildStatusSet(("CORE-1", "done", []), ("CORE-2", "todo", ["CORE-1"]))), "CORE") + + assert cullGraph(graph, 1).graph.scope == "CORE" diff --git a/tests/test_handoff.py b/tests/test_handoff.py index b3152dd..6701009 100644 --- a/tests/test_handoff.py +++ b/tests/test_handoff.py @@ -13,9 +13,10 @@ import pytest from docket.core.config import Config -from docket.core.handoff import HANDOFF_TEMPLATE, KeyBriefing, buildContext, buildKeyBriefings, readDocument, renderHandoff +from docket.core.handoff import HANDOFF_TEMPLATE, KeyBriefing, buildContext, buildKeyBriefings, renderHandoff from docket.core.ids import buildFilename, isValidId from docket.core.store import Store +from docket.core.templating import readDocument from docket.core.ticket import STATUSES, Ticket, parseTicket from docket.core.titles import isTitleCase diff --git a/tests/test_packaging.py b/tests/test_packaging.py index f6678f8..f635e5f 100644 --- a/tests/test_packaging.py +++ b/tests/test_packaging.py @@ -34,6 +34,7 @@ "templates/CLAUDE.md", "templates/docket.toml", "docs/writingTicketsOffsite.md.jinja", + "docs/roadmap.md.jinja", ) # Directories that must never reach the source distribution, because they are development state rather than source. diff --git a/tests/test_roadmap.py b/tests/test_roadmap.py new file mode 100644 index 0000000..d55b42d --- /dev/null +++ b/tests/test_roadmap.py @@ -0,0 +1,173 @@ +""" +Roadmap Tests + +Cover the rendered document's shape, its scoped heading, its legend, and what the node ceiling costs it. +""" + +# MARK: Imports + +import pytest + +from docket.core.roadmap import ROADMAP_TEMPLATE, Roadmap, buildRoadmap +from docket.core.store import Store +from docket.core.templating import readDocument +from docket.core.ticket import Ticket + +# MARK: Constants + +# The fence a markdown renderer needs in order to draw the diagram rather than print it. +MERMAID_FENCE: str = "```mermaid\n" + +# MARK: Functions + + +def seed(store: Store) -> None: + """ + Create a small set of tickets with a dependency crossing between two keys. + + store: The store to create them in. + """ + + store.create(key="CORE", title="App Shell") + store.create(key="GEN", title="Map Generation", requires=["CORE-1"]) + + +def testDocumentFencesTheDiagram(store: Store) -> None: + """ + The whole point of the document over bare mermaid source is that a markdown renderer draws the diagram, which takes a fence the renderer itself never emits. + """ + + seed(store) + + document: str = buildRoadmap(store).document + + assert document.startswith("# Roadmap\n") + assert MERMAID_FENCE in document + + # The diagram sits inside the fence rather than beside it, and the fence closes. + body: str = document.split(MERMAID_FENCE, 1)[1] + + assert body.startswith("graph TD\n") + assert "\n```\n" in body + + +def testDocumentCarriesTheLegend(store: Store) -> None: + """ + A renderer that ignores `classDef` drops every fill and border, so the shapes have to be explained in text that survives anywhere. + """ + + seed(store) + + document: str = buildRoadmap(store).document + + assert "## Legend" in document + + for shape in ("`[ ]`", "`{ }`", "`( )`"): + assert shape in document + + +def testDocumentIsUnchangedBetweenRuns(store: Store) -> None: + """ + The document is committed, so anything that differed between two runs over the same tickets would show up as a diff that means nothing. + """ + + seed(store) + + assert buildRoadmap(store).document == buildRoadmap(store).document + + +def testUnscopedDocumentNamesNoScope(store: Store) -> None: + """ + The plain heading is what says the document covers the whole repository. + """ + + seed(store) + + assert buildRoadmap(store).document.startswith("# Roadmap\n") + + +@pytest.mark.parametrize( + ("kwargs", "heading"), + [ + ({"key": "CORE"}, "# Roadmap: CORE\n"), + ({"status": "todo"}, "# Roadmap: todo\n"), + ({"ticketId": "CORE-1"}, "# Roadmap: CORE-1\n"), + ], +) +def testScopedDocumentNamesItsScope(store: Store, kwargs: dict[str, str], heading: str) -> None: + """ + A scoped roadmap says so in its own heading, so it cannot be mistaken for the whole project. + + kwargs: The scope to build with. + heading: The heading the scope should produce. + """ + + seed(store) + + assert buildRoadmap(store, **kwargs).document.startswith(heading) + + +def testLegendExplainsABorrowedTicketOnlyWhenOneIsShown(store: Store) -> None: + """ + A key is the one scope that borrows from outside itself, so the dashed border is explained there and nowhere else. + """ + + seed(store) + + assert "A dashed border" in buildRoadmap(store, key="CORE").document + assert "A dashed border" not in buildRoadmap(store).document + + +def testCeilingDropsFinishedWorkAndIsReported(store: Store) -> None: + """ + The document deliberately says nothing about what was dropped, so the count has to leave through the result for the caller to report instead. + """ + + seed(store) + + # Finish the dependency, leaving one open ticket and one completed one. + finished: Ticket = store.setStatus("CORE-1", "done") + + assert finished.isDone + + roadmap: Roadmap = buildRoadmap(store, maxNodes=1) + + assert roadmap.dropped == 1 + assert "CORE-1" not in roadmap.document + assert "GEN-1" in roadmap.document + + # Nothing in the file admits to the omission, which is the whole reason the count travels beside it. + assert "omitted" not in roadmap.document + + +def testNoCeilingKeepsEverything(store: Store) -> None: + """ + Zero is the documented way to ask for the whole graph, which is also the default every caller passes when no ceiling is configured. + """ + + seed(store) + + roadmap: Roadmap = buildRoadmap(store, maxNodes=0) + + assert roadmap.dropped == 0 + assert "CORE-1" in roadmap.document + assert "GEN-1" in roadmap.document + + +def testAnEmptyRepositoryStillRenders(store: Store) -> None: + """ + A freshly deployed repository has no tickets, and a roadmap run there is a reasonable thing to do rather than an error. + """ + + document: str = buildRoadmap(store).document + + assert document.startswith("# Roadmap\n") + assert MERMAID_FENCE in document + + +def testTemplateHoldsNoEmDash() -> None: + """ + The repository writes no em dash anywhere, and a shipped document is read by more people than the source is. + """ + + assert "—" not in readDocument(ROADMAP_TEMPLATE)