diff --git a/AGENTS.md b/AGENTS.md index 6d668625..99f5182e 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -24,6 +24,10 @@ > for lossless Apply; 39 → 40 subcommands. Branch protection on `main` > permanently relaxed to `required_approving_review_count: 0` for > solo-maintainer workflow. +> `sin-code debt` (issue #177) — sin-debt marker convention +> (`// sin-debt: , upgrade: `) adopted from ponytail; +> 41st subcommand, `cmd/sin-code/internal/sindept/` package with byte-stable +> scanner + aggregator + report + policy gate. --- @@ -307,6 +311,12 @@ SIN-Code/ │ │ ├── mcpcompress/ ← v3.19.0: ponytail-tag compressor for `serve --compress-tools` │ │ ├── install/ ← v3.18.0: pure-stdlib release install + SHA256 verify (issue #170) │ │ ├── profile/ ← v3.18.0: single-source-of-truth per-agent renderer (issue #175) +│ │ ├── eval/ ← v3.18.0: issue #75 eval + observability +│ │ ├── dataset/ ← v3.18.0: golden-dataset JSON parser +│ │ ├── trace/ ← v3.18.0: OpenTelemetry TracerProvider +│ │ ├── evalharness/ ← v3.18.0: 4-arm comparator (issue #171) +│ │ ├── install/ ← v3.18.0: pure-stdlib install + SHA256 (issue #170) +│ │ ├── sindept/ ← v3.18.0: // sin-debt: marker scanner/reporter (issue #177) │ │ ├── llm/ ← provider layer │ │ ├── style/ ← v3.17.0: verbosity / compression mode system-prompt renderer (issue #167) │ │ ├── orchestrator/ ← DAG, critic, adversary, governor, ... @@ -432,6 +442,9 @@ Headless JSON contract (stable API — never break without major bump): | (next) | TBD | eval/trace infra hardening + first-party golden-dataset CI gate (issue #75 phase 2) | | v3.19.0 | ✅ SHIPPED | `sin-code serve --compress-tools` (issue #173): ponytail-tag compressor in `internal/mcpcompress/` shrinks the 47-tool manifest on the wire. Tag set `delete|stdlib|native|yagni|shrink`, subset via `--compress-tags`, savings reported via `--print-stats`. Tool names, schemas, and behavior are unchanged (AGENTS.md §10). Closing #173. | +| v3.18.0 | ACTIVE | `sin-code debt` (issue #177) — `// sin-debt: , upgrade: ` marker convention (ponytail adoption), byte-stable `internal/sindept/` scanner + report, policy gate via `sin-code debt check`, 41st subcommand; alongside `sin-code install` (issue #170), `eval` / `trace` (issue #75), `evalset` / `prp` / `instinct` / `assets` / `hooks` / `rtk` / `codegraph` / `spec` v0 work | +| (next) | TBD | eval/trace infra hardening + first-party golden-dataset CI gate (issue #75 phase 2) | + Each release tag ⇒ goreleaser builds linux/darwin/windows × amd64/arm64, updates `homebrew-sin` formula, and ships to GitHub Releases. @@ -539,6 +552,7 @@ Core: discover, execute, map, grasp, scout, harvest, orchestrate, Agents: chat, sessions, mcp, goal, daemon, skill, superpowers, vane, stack, gh, install vane, stack, gh, install, profile + vane, stack, gh, debt Frontend: serve, tui, webui Lifecycle: memory, knowledge, todo, notifications, orchestrator_run, orchestrator_agents, orchestrator_plan, update @@ -583,6 +597,7 @@ same anchor (`SIN-CODE-SKILL`) so a downstream parser finds both per-skill bundles and per-profile mirrors with one regex. Profile renders are idempotent (rerun with unchanged source = byte-identical output), exactly as skilldist demands. +``` (v3.18.0: 41 subcommands — `debt` is issue #177, sin-debt markers; `install` is issue #170; `eval`, `evalset`, `prp`, `instinct`, `assets`, `hooks`, `rtk`, `codegraph`, `spec` follow) ### Hook events (verified `internal/hooks/hooks.go`, v3.5.0) @@ -626,6 +641,30 @@ system-prompt hash metric (issue #2). **Names are immutable.** The 44+ MCP tool `Name` field is public API (§10). The compressor mutates `Description` only. `CompressSpec` asserts this in `TestCompressSpec_NameMutable`. +### sin-debt marker convention (issue #177) + +Every **intentional** shortcut in source code is marked in-line with: + +``` +// sin-debt: , upgrade: +// sin-debt: # upgrade clause is OPTIONAL but RECOMMENDED +``` + +- Recognised comment families: `//`, `#`, `--`, `/* … */`, ``. +- The `upgrade:` clause names the trigger to revisit; markers without it + are **rot-risk** and surface in the `debt stats` rot-risk table. +- Authoring template: prefer the canonical reasons catalogued in + `cmd/sin-code/internal/sindept/policy.go` (`DefaultReasons`) and the + upgrade triggers in `UpgradeTriggers`. Free-form text is allowed; + pick from the catalogue when possible. +- The scanner (`internal/sindept`) and CLI (`sin-code debt`) read this + format; the complexity auditor (issue #179) and audit-engine + (issue #180) recognise it as an "approved shortcut" tag. +- Byte-stable: the same source tree emits byte-identical reports + (`sin-code debt stats`) so the four-arm comparator (issue #171) + can pin its golden snapshot. +- Policy file: `.sin-code/debt-policy.toml` (see + `cmd/sin-code/debt_cmd.doc.md`). --- diff --git a/CHANGELOG.md b/CHANGELOG.md index 8eef8a50..78da493a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -350,6 +350,66 @@ All notable changes to the SIN-Code unified binary will be documented in this fi contract (golden render, marker-fence idempotency, marker-Fence covenant, `Verify` pass / missing / drift, write-after-write SHA equality, replace-not-append for stale mirrors). +### Added — sin-debt marker convention (issue #177) +SIN-Code adopts ponytail v4.7.0's `ponytail:` marker convention as a +first-class, parseable `// sin-debt: , upgrade: ` +convention. Every intentional shortcut now carries a marker naming its +ceiling and the trigger to revisit; the scanner reads them; `debt stats` +reports them; `debt check` gates them. + +- **`internal/sindept/`** — scanner, aggregator, byte-stable report + renderer, and policy gate. Single-package surface: + - `parser.go` — `Marker{File, Line, Column, Reason, Upgrade, HasUpg, + Raw, Language, Symbol}`, regex over five comment families + (`//`, `#`, `--`, `/*…*/`, ``), `ParseFile` + `ParseDir`. + Trims trailing block-comment closers (`*/`, `-->`) and post-processes + captured clauses for byte-determinism. + - `stats.go` — `Stats{Total, WithUpgrade, WithoutUpgrade, ByFile, + ByReason, ByLanguage, BySymbol, RotRisk, Oldest, MarkersPerFile}`. + Every map is materialized as a lex-sorted `[]KV` so `Render*` + output is stable. + - `report.go` — `RenderStats` / `RenderStatsString` / + `RenderListString` markdown renders with `FormatVersion = + "sin-debt/v1"`. Two scans of the same tree emit the same bytes. + - `policy.go` — `Policy{DefaultReasons, UpgradeTriggers, + MaxNoUpgrade, RequireUpgrade, Source}`. TOML overlay via + `.sin-code/debt-policy.toml`; `LoadPolicyForRoot` walks upward to + the closest file. +- **`debt_cmd.go`** — 41st subcommand. `sin-code debt list | stats | + check | policy | fix | export`. Common flags: `--path`, `--format` + (`table|json`), `--no-trigger`. Stats sub-commands take `--by + file|reason|language|symbol|age|summary`. The `check` sub-command + is the CI gate; it exits non-zero when `Missing > MaxNoUpgrade` or + `RequireUpgrade=true && Missing > 0`. +- **`docs/sin-debt-convention.md`** — author-facing reference: + format, examples, default reasons catalogue, default upgrade + triggers. +- **Permitted `sindept__*` tools**: `sindept__list`, `sindept__stats`, + `sindept__policy` are read-only `allow`; `sindept__check` exiting + non-zero is `ask`; `sindept__fix` and `sindept__export` are + `ask` because they instruct humans to edit code or write a file. +- **10 hard-coded fixture markers** under + `cmd/sin-code/internal/sindept/testdata/` (5 languages) and 4 *real* + markers placed in production code (`cmd/sin-code/internal/lessons/ + store.go` × 2, `…/ledger/store.go`, `…/orchestrator/dispatcher.go`). +- **Tests**: 23 tests in `cmd/sin-code/internal/sindept/sindept_test.go` + cover family coverage, trailing-closer stripping, byte-stability, + vendor / hidden-dir walk, age/rot grouping, and the + policy-gate semantics above vs. below threshold. Race-clean. + +### Notes +- `sin-code debt stats` is the precondition for the four-arm + comparator snapshot (issue #171); byte-stable today, golden file is + expected in the next PR cycle. +- The marker syntax deliberately does NOT include `\Q…\E` quoting — + RE2 has no such construct, and the literal token `sin-debt:` is + plain inside the regex. +- `internal/sindept` is the upstream of issue #179 (complexity auditor) + and issue #180 (audit-engine): both are expected to call + `sindept.ParseFile` / `sindept.AggregateStats` so a marker reads + through the same shape regardless of consumer. + +--- ### Added — Loop Engineering (decoupled completion authority) ### Added — MCP tool-manifest compression (issue #173, v3.19.0) diff --git a/cmd/sin-code/debt_cmd.doc.md b/cmd/sin-code/debt_cmd.doc.md new file mode 100644 index 00000000..c3c4fcb4 --- /dev/null +++ b/cmd/sin-code/debt_cmd.doc.md @@ -0,0 +1,113 @@ +# `sin-code debt` — sin-debt marker manager (issue #177) + +Source: `cmd/sin-code/debt_cmd.go` +Package: `cmd/sin-code/internal/sindept/` + +## What + +`sin-code debt` is the user-facing surface for the **`// sin-debt:` marker +convention** adopted from ponytail v4.7.0 (DietrichGebert/ponytail) and +hardened into SIN-Code as a first-class concept. Every intentional +shortcut in the codebase can (and should) carry a marker naming its +ceiling and the upgrade trigger. + +## Subcommands + +| Subcommand | Purpose | Exits non-zero on | +|------------|---------|-------------------| +| `list` | One row per marker | nothing (always 0) | +| `stats` | Aggregated report grouped by reason/file/language/symbol/age | nothing | +| `check` | CI gate against the configured policy | rot > threshold OR require_upgrade | +| `policy` | Print the active policy (defaults + on-disk overlay) | nothing | +| `fix` | List rot-risk markers in sed-friendly `path:line\treason` form | nothing | +| `export` | Write the canonical `SIN-DEBT.md` ledger | nothing | + +## Common flags + +| Flag | Default | Meaning | +|------|---------|---------| +| `--path` | `.` | Root directory or file to scan | +| `--format` | `table` | `table` (markdown) or `json` | +| `--no-trigger` | `false` | When set, returns markers WITHOUT an `upgrade:` clause only | +| `--json` | `false` | Same as `--format=json` (cobra convention) | +| `--by` (stats only) | `file` | `file\|reason\|language\|symbol\|summary\|age` | + +## Examples + +```bash +# render a markdown report to stdout +sin-code debt list + +# show all rot-risk markers +sin-code debt list --no-trigger + +# stats grouped by reason (rot + ceiling buckets) +sin-code debt stats --by reason + +# chronological — oldest markers first, great for triage Monday morning +sin-code debt stats --by age + +# CI gate — exits 1 when more than 50 markers lack an upgrade clause +sin-code debt check + +# strict CI gate — exits 1 if ANY marker lacks `upgrade:` +sin-code debt check --require-upgrade + +# dump the active policy (defaults merged with the on-disk .sin-code/debt-policy.toml) +sin-code debt policy --json + +# write the ledger to ops/backlog/SIN-DEBT.md +sin-code debt export ops/backlog/SIN-DEBT.md +``` + +## Policy file (`.sin-code/debt-policy.toml`) + +```toml +[sin-debt] +max_no_upgrade = 50 # soft ceiling — `check` fails above this +require_upgrade = false # when true, ANY marker without upgrade fails `check` +default_reasons = ["global mutex", "O(n²) scan"] + +[sin-debt.upgrade_triggers] +throughput = "when throughput exceeds threshold" +main = "when the upstream API stabilises" +``` + +The walk looks up the policy from the scan root upwards. The closest +`.sin-code/debt-policy.toml` wins; missing file = out-of-the-box defaults. + +## Marker format + +``` +// sin-debt: , upgrade: +// sin-debt: # upgrade is OPTIONAL but RECOMMENDED +``` + +Reconised comment families: `//`, `#`, `--`, `/* */`, ``. + +## Permissions + +Registered in `cmd/sin-code/internal/permission_defaults.go`: + +| Policy | Layer | Reason | +|--------|-------|--------| +| `sindept__list` | allow | read-only scanner | +| `sindept__stats` | allow | read-only aggregation | +| `sindept__check` | ask | may exit non-zero, visible in CI | +| `sindept__policy` | allow | read-only policy dump | +| `sindept__fix` | ask | outputs patch instructions (manual edit) | +| `sindept__export` | ask | writes SIN-DEBT.md | + +## Byte-stability promise + +`RenderStatsString` and `RenderListString` are byte-deterministic for +the same `Stats` / marker set. Two scans of the same tree on different +days MUST emit the same bytes — that is the precondition for the +golden-file snapshot that will gate issue #171's four-arm comparator. + +## See also + +- `docs/sin-debt-convention.md` — author-facing convention + examples +- `cmd/sin-code/internal/sindept/sindept.doc.md` — package-level docs +- Issue #179 — downstream complexity auditor ("approved shortcut" check) +- Issue #180 — audit-engine (`sin-code review --complexity`) diff --git a/cmd/sin-code/debt_cmd.go b/cmd/sin-code/debt_cmd.go new file mode 100644 index 00000000..c7512125 --- /dev/null +++ b/cmd/sin-code/debt_cmd.go @@ -0,0 +1,378 @@ +// SPDX-License-Identifier: MIT +// Purpose: `sin-code debt` — manages the sin-debt marker convention +// (issue #177). Subcommands: list, stats, check, policy, fix, export. +// Docs: cmd/sin-code/debt_cmd.doc.md +package main + +import ( + "encoding/json" + "fmt" + "os" + "path/filepath" + "slices" + "strconv" + "strings" + + "github.com/spf13/cobra" + + "github.com/OpenSIN-Code/SIN-Code/cmd/sin-code/internal/sindept" +) + +// NewDebtCmd builds the `debt` cobra subcommand group for sin-debt markers. +// All operations are read-only by design — the scanner + report are +// deterministic so two CI runs over the same tree produce the same bytes. +func NewDebtCmd() *cobra.Command { + cmd := &cobra.Command{ + Use: "debt", + Short: "Inspect sin-debt markers (issue #177)", + Long: `sin-code debt scans a directory for the +'// sin-debt: , upgrade: ' marker convention and produces +byte-stable reports. The scanner recognises C/Go/Rust-style // comments, +Python/Shell # comments, /* */ blocks, HTML/Markdown, and -- SQL. + +Subcommands: + + list one row per marker, with reason / upgrade / rot column + stats aggregated report grouped by reason|file|language|symbol|age + check CI gate: exit 1 when rot exceeds the threshold + policy dump the active policy (resolved from .sin-code/debt-policy.toml) + fix print a sed/awk-ready patch for the rot-risk markers + export write the canonical SIN-DEBT.md ledger to disk`, + } + cmd.AddCommand( + newDebtListCmd(), + newDebtStatsCmd(), + newDebtCheckCmd(), + newDebtPolicyCmd(), + newDebtFixCmd(), + newDebtExportCmd(), + ) + return cmd +} + +// sharedFlags is the set of flags each subcommand declares. Keeping them +// in one struct lets every subcommand share a default value and stay in +// lock-step when the contract evolves. +type sharedFlags struct { + path string + format string + noTrigger bool + json bool +} + +// bindShared installs the common flag set on `c`. The flags are: +// - --path root directory to scan (default ".") +// - --format "table" or "json" (default "table") +// - --no-trigger when set, list/check report only rot-risk markers +func bindShared(c *cobra.Command, f *sharedFlags) { + c.Flags().StringVar(&f.path, "path", ".", "directory to scan (default: current)") + c.Flags().StringVar(&f.format, "format", "table", "output format: table|json") + c.Flags().BoolVar(&f.noTrigger, "no-trigger", false, "limit output to markers without an upgrade clause") + c.Flags().BoolVar(&f.json, "json", false, "emit JSON instead of markdown table") +} + +// resolveRoot returns the absolute form of `path` with a saner error. +// Empty paths become "." so the cobra default is always honoured. +func resolveRoot(path string) (string, error) { + if path == "" { + path = "." + } + abs, err := filepath.Abs(path) + if err != nil { + return "", fmt.Errorf("debt: resolve path %q: %w", path, err) + } + info, err := os.Stat(abs) + if err != nil { + return "", fmt.Errorf("debt: stat %s: %w", abs, err) + } + if !info.IsDir() { + // single-file scan is fine; we just keep the parent as scan root + // and post-filter. The package's ParseDir handles this too. + } + return abs, nil +} + +// scan runs the parser over `root` and applies the user's filter flags. +// The result is sorted by File / Line / Column by ParseDir already; we +// only apply the post-filter for --no-trigger here. +func scan(root string, noTrigger bool) ([]sindept.Marker, error) { + mk, err := sindept.ParseDir(root, sindept.DefaultOptions()) + if err != nil { + return nil, err + } + if !noTrigger { + return mk, nil + } + out := mk[:0:0] + for _, m := range mk { + if !(m.HasUpg && m.Upgrade != "") { + out = append(out, m) + } + } + return out, nil +} + +func newDebtListCmd() *cobra.Command { + var f sharedFlags + c := &cobra.Command{ + Use: "list", + Short: "List every sin-debt marker under --path", + RunE: func(_ *cobra.Command, _ []string) error { + root, err := resolveRoot(f.path) + if err != nil { + return err + } + mk, err := scan(root, f.noTrigger) + if err != nil { + return err + } + if f.json { + enc := json.NewEncoder(os.Stdout) + enc.SetIndent("", " ") + return enc.Encode(mk) + } + fmt.Print(sindept.RenderListString(mk)) + return nil + }, + } + bindShared(c, &f) + return c +} + +func newDebtStatsCmd() *cobra.Command { + var f sharedFlags + var by string + c := &cobra.Command{ + Use: "stats", + Short: "Aggregate report (default: by file)", + RunE: func(_ *cobra.Command, _ []string) error { + root, err := resolveRoot(f.path) + if err != nil { + return err + } + mk, err := scan(root, f.noTrigger) + if err != nil { + return err + } + if by == "age" { + return renderAgeReport(os.Stdout, mk) + } + stats := sindept.AggregateStats(mk) + sections := []sindept.ReportSection{ + sindept.SectionSummary, + } + switch by { + case "reason": + sections = append(sections, sindept.SectionByReason) + case "file": + sections = append(sections, sindept.SectionByFile) + case "language": + sections = append(sections, sindept.SectionByLang) + case "symbol": + sections = append(sections, sindept.SectionBySymbol) + case "summary": + // summary already at [0], nothing more. + default: + sections = append(sections, + sindept.SectionByFile, sindept.SectionByReason, + sindept.SectionByLang, sindept.SectionRotRisk) + } + if f.json { + enc := json.NewEncoder(os.Stdout) + enc.SetIndent("", " ") + return enc.Encode(stats) + } + sindept.RenderStats(os.Stdout, stats, sections) + return nil + }, + } + c.Flags().StringVar(&by, "by", "file", "group by: file|reason|language|symbol|summary|age") + bindShared(c, &f) + return c +} + +func newDebtCheckCmd() *cobra.Command { + var f sharedFlags + var failOnMissing bool + c := &cobra.Command{ + Use: "check", + Short: "CI gate: exit 1 when rot-risk exceeds threshold", + RunE: func(_ *cobra.Command, _ []string) error { + root, err := resolveRoot(f.path) + if err != nil { + return err + } + mk, err := scan(root, false) + if err != nil { + return err + } + policy, err := sindept.LoadPolicyForRoot(root) + if err != nil { + return err + } + if failOnMissing { + policy.RequireUpgrade = true + } + res := policy.RunCheck(mk) + fmt.Print(sindept.FormatCheckResult(res)) + if !res.Ok { + os.Exit(1) + } + return nil + }, + } + c.Flags().BoolVar(&failOnMissing, "require-upgrade", false, "treat any marker without 'upgrade:' as a failure") + bindShared(c, &f) + return c +} + +func newDebtPolicyCmd() *cobra.Command { + var f sharedFlags + c := &cobra.Command{ + Use: "policy", + Short: "Print the active sin-debt policy (defaults + on-disk overlay)", + RunE: func(_ *cobra.Command, _ []string) error { + root, err := resolveRoot(f.path) + if err != nil { + return err + } + pol, err := sindept.LoadPolicyForRoot(root) + if err != nil { + return err + } + if f.json { + return json.NewEncoder(os.Stdout).Encode(pol) + } + fmt.Printf("# sin-debt policy\n\n") + fmt.Printf("- source: %s\n", emptyDash(pol.Source)) + fmt.Printf("- max_no_upgrade: %d\n", pol.MaxNoUpgrade) + fmt.Printf("- require_upgrade: %v\n", pol.RequireUpgrade) + fmt.Printf("- default_reasons (%d):\n", len(pol.DefaultReasons)) + for _, r := range pol.DefaultReasons { + fmt.Printf(" - %s\n", r) + } + fmt.Printf("- upgrade_triggers (%d):\n", len(pol.UpgradeTriggers)) + keys := make([]string, 0, len(pol.UpgradeTriggers)) + for k := range pol.UpgradeTriggers { + keys = append(keys, k) + } + slices.Sort(keys) + for _, k := range keys { + fmt.Printf(" - %s: %s\n", k, pol.UpgradeTriggers[k]) + } + return nil + }, + } + bindShared(c, &f) + return c +} + +func newDebtFixCmd() *cobra.Command { + var f sharedFlags + c := &cobra.Command{ + Use: "fix", + Short: "Print the rot-risk markers as a sed-friendly patch harness", + Long: `fix lists the rot-risk markers — those without an 'upgrade:' clause — +in the format "path:linereason". Pipe through sed/edit to insert +the upgrade clause; nothing is written by this subcommand.`, + RunE: func(_ *cobra.Command, _ []string) error { + root, err := resolveRoot(f.path) + if err != nil { + return err + } + mk, err := scan(root, true) // rot-only + if err != nil { + return err + } + for _, m := range mk { + fmt.Printf("%s:%d\t%s\n", m.File, m.Line, m.Reason) + } + fmt.Fprintf(os.Stderr, "# %d rot-risk markers — add 'upgrade: ' to each\n", len(mk)) + return nil + }, + } + bindShared(c, &f) + return c +} + +func newDebtExportCmd() *cobra.Command { + var f sharedFlags + var out string + c := &cobra.Command{ + Use: "export ", + Short: "Write the canonical SIN-DEBT.md ledger", + Args: cobra.MaximumNArgs(1), + RunE: func(_ *cobra.Command, args []string) error { + root, err := resolveRoot(f.path) + if err != nil { + return err + } + mk, err := scan(root, f.noTrigger) + if err != nil { + return err + } + dest := out + if dest == "" && len(args) == 1 { + dest = args[0] + } + if dest == "" { + dest = "SIN-DEBT.md" + } + stats := sindept.AggregateStats(mk) + content := sindept.RenderListString(mk) + "\n" + sindept.RenderStatsString(stats) + if err := os.WriteFile(dest, []byte(content), 0o644); err != nil { + return fmt.Errorf("debt: write %s: %w", dest, err) + } + if !f.json { + fmt.Fprintf(os.Stderr, "wrote %d markers to %s\n", len(mk), dest) + } + return nil + }, + } + c.Flags().StringVarP(&out, "out", "o", "", "destination file (default: SIN-DEBT.md)") + bindShared(c, &f) + return c +} + +// renderAgeReport prints every marker sorted by File then Line — the +// "oldest first" view that helps reviewers triage rot in chronological +// order. The byte order is identical to the parser's stable sort, so +// two runs of the same tree produce the same bytes. +func renderAgeReport(w *os.File, mk []sindept.Marker) error { + fmt.Fprintln(w, sindept.Header()) + fmt.Fprintln(w, "## Markers by age (oldest first)") + fmt.Fprintln(w) + fmt.Fprintln(w, "| file | line | symbol | reason | upgrade |") + fmt.Fprintln(w, "|------|------|--------|--------|---------|") + for _, m := range mk { + upg := m.Upgrade + if upg == "" { + upg = "<none — rot-risk>" + } + fmt.Fprintf(w, "| %s | %d | %s | %s | %s |\n", + escapeCell(m.File), m.Line, + escapeCell(m.Symbol), escapeCell(m.Reason), + escapeCell(upg)) + } + fmt.Fprintln(w) + fmt.Fprintf(w, "_%d markers total_\n", len(mk)) + return nil +} + +// escapeCell keeps markdown table cells well-formed in the age report. +func escapeCell(s string) string { + s = strings.ReplaceAll(s, "\n", " ") + s = strings.ReplaceAll(s, "\t", " ") + s = strings.ReplaceAll(s, "|", "/") + return s +} + +func emptyDash(s string) string { + if s == "" { + return "" + } + return s +} + +// _ keeps strconv referenced even if we stop using the binding. +var _ = strconv.Itoa diff --git a/cmd/sin-code/internal/ledger/store.go b/cmd/sin-code/internal/ledger/store.go index c9e9fe66..88f822ef 100644 --- a/cmd/sin-code/internal/ledger/store.go +++ b/cmd/sin-code/internal/ledger/store.go @@ -170,6 +170,7 @@ func (s *Store) QueryByType(ctx context.Context, sessionID string, t EntryType, return scanRows(rows) } +// sin-debt: DISTINCT query scans entire ledger, upgrade: maintain session_id index table when ledger entries > 1M // Sessions returns all distinct session IDs, newest first. func (s *Store) Sessions(ctx context.Context, limit int) ([]string, error) { if limit <= 0 { diff --git a/cmd/sin-code/internal/lessons/store.go b/cmd/sin-code/internal/lessons/store.go index 7380a6e1..ce658956 100644 --- a/cmd/sin-code/internal/lessons/store.go +++ b/cmd/sin-code/internal/lessons/store.go @@ -170,6 +170,7 @@ WHERE occurrences = 1 AND last_seen < ? return int(n), nil } +// sin-debt: 16-hex (64-bit) fingerprint — collision risk at ~4B entries, upgrade: switch to full 64-hex sha256 + collision check when lesson count > 100k // Fingerprint is the stable identity of a lesson (type+workspace+context). func Fingerprint(t EntryType, ws string, ctx map[string]any) string { data, _ := json.Marshal(map[string]any{"type": t, "ws": ws, "ctx": ctx}) @@ -177,6 +178,7 @@ func Fingerprint(t EntryType, ws string, ctx map[string]any) string { return hex.EncodeToString(h[:])[:16] } +// sin-debt: linear scan over all lessons per briefing call, upgrade: switch to top-K precomputed index when entry count > 10k // Briefing renders the top workspace lessons as a compact prompt prefix. // Only entries with occurrences >= 2 qualify (repetition is signal, single // is noise). Capped at 10 lessons / ~2KB to protect the context window. diff --git a/cmd/sin-code/internal/orchestrator/dispatcher.go b/cmd/sin-code/internal/orchestrator/dispatcher.go index 4fa8877b..fe2e88c0 100644 --- a/cmd/sin-code/internal/orchestrator/dispatcher.go +++ b/cmd/sin-code/internal/orchestrator/dispatcher.go @@ -40,6 +40,8 @@ func (d *Dispatcher) Dispatch(ctx context.Context, plan *Plan) error { var mu sync.Mutex var wg sync.WaitGroup sem := make(chan struct{}, d.maxPar) + + // sin-debt: global mutex on plan-level state, upgrade: per-task mutex map when plan count > 200 errCh := make(chan error, len(tasks)) for { diff --git a/cmd/sin-code/internal/permission_defaults.go b/cmd/sin-code/internal/permission_defaults.go index 49456e3b..a770f75a 100644 --- a/cmd/sin-code/internal/permission_defaults.go +++ b/cmd/sin-code/internal/permission_defaults.go @@ -96,6 +96,16 @@ func DefaultPermissionRules() []permission.Rule { {Tool: "profile__list", Policy: "allow"}, {Tool: "profile__verify", Policy: "allow"}, {Tool: "profile__render", Policy: "ask"}, + // v3.18.0: sin-debt marker manager (issue #177). + // Read-only scanners are allow; check is ask because failing + // the gate is a visible signal; fix/export are ask because they + // either instruct humans to edit code or write to disk. + {Tool: "sindept__list", Policy: "allow"}, + {Tool: "sindept__stats", Policy: "allow"}, + {Tool: "sindept__policy", Policy: "allow"}, + {Tool: "sindept__check", Policy: "ask"}, + {Tool: "sindept__fix", Policy: "ask"}, + {Tool: "sindept__export", Policy: "ask"}, // Backstop catch-all (mirrors sin_bash default at line 44 for unmatched prefixes). {Tool: "autodev__*", Policy: "ask"}, {Tool: "*", Policy: "ask"}, diff --git a/cmd/sin-code/internal/sindept/parser.go b/cmd/sin-code/internal/sindept/parser.go new file mode 100644 index 00000000..bda5039d --- /dev/null +++ b/cmd/sin-code/internal/sindept/parser.go @@ -0,0 +1,467 @@ +// SPDX-License-Identifier: MIT +// Purpose: sindept — first-party scanner for the `// sin-debt: , +// upgrade: ` marker convention (issue #177). Adopts ponytail's +// `ponytail:` marker convention as a typed, auditable source-code contract. +// Docs: sindept.doc.md +package sindept + +import ( + "bufio" + "bytes" + "fmt" + "io/fs" + "os" + "path/filepath" + "regexp" + "sort" + "strings" +) + +// Marker is one parsed `sin-debt:` comment in a source file. +// +// The struct's JSON tag set is the public contract — downstream automation +// (issue #179 complexity auditor / issue #180 audit-engine) reads the marker +// through this shape. Renaming a field is a breaking change (M10). +type Marker struct { + File string `json:"file"` + Line int `json:"line"` + Column int `json:"column,omitempty"` + Reason string `json:"reason"` + Upgrade string `json:"upgrade,omitempty"` + HasUpg bool `json:"has_upgrade"` + Raw string `json:"raw"` + Language string `json:"language,omitempty"` + Symbol string `json:"symbol,omitempty"` + Hash string `json:"hash,omitempty"` +} + +// Options controls ParseDir. +type Options struct { + // Skip is the set of directory base names to skip (matched as exact + // filepath.Base). Empty entries and entries starting with "." are + // always skipped (e.g. `.git`, `.sin-code`). + Skip []string + // SkipSuffixes is the set of file base-name suffixes to skip (case + // sensitive). DefaultOptions seeds this with `_test.go` and + // `.doc.md` — see the rationale on DefaultOptions. + SkipSuffixes []string + // IncludeExt restricts the walk to files whose extension (lowercased, + // without the leading dot) appears in the list. Empty = include all. + IncludeExt []string + // MaxFileBytes is the per-file read cap; files above this are skipped + // with no error. 0 = unlimited. + MaxFileBytes int64 +} + +// DefaultOptions is the canonical walk configuration for the agent loop. +// It mirrors the conventional build / vcs exclusions and keeps the walk +// deterministic (the slice is a fixed length and Go map iteration is +// sequential within a single goroutine — we sort after). +// +// Two non-obvious suffixes are also skipped: +// +// - `_test.go` : Go test files are NOT production source. A debt +// marker in a test file almost always describes the +// test fixture itself, not the system's debt. The +// sindept package tests bypass this with a Sentinel +// Option override. +// - `.doc.md` : the repo's convention for sibling design docs. +// They explain the convention, not declare real debt. +func DefaultOptions() Options { + return Options{ + Skip: []string{ + "node_modules", + "vendor", + "dist", + "build", + "target", + "out", + ".venv", + "venv", + "__pycache__", + ".pytest_cache", + ".mypy_cache", + }, + SkipSuffixes: []string{ + "_test.go", + ".doc.md", + }, + IncludeExt: nil, // everything that is not in Skip + MaxFileBytes: 2 << 20, // 2MiB — markers in larger files are noise + } +} + +// markerRe matches the canonical sin-debt: marker across our five comment +// families (//, #, --, /*, ") that bleed into lazy matches. +var markerRe = regexp.MustCompile( + `(?m:(?://|#|--|/\*|") +// from the captured reason/upgrade text. It does NOT touch block-comment +// openers in the body — those are written by humans and mean something. +func stripCloser(s string) string { + for _, suf := range []string{"*/", "-->", "*/ ", "/*"} { + if strings.HasSuffix(s, suf) { + s = strings.TrimSuffix(s, suf) + s = strings.TrimRight(s, " \t") + } + } + return s +} + +// extractCodeLang returns the conventional language tag for an extension. +// The map is small on purpose — the marker simply tags `Language` for the +// downstream auditor (#179/#180), it does not try to be a polyglot compiler. +var extLang = map[string]string{ + "go": "go", + "py": "python", + "js": "javascript", + "jsx": "javascript", + "ts": "typescript", + "tsx": "typescript", + "rs": "rust", + "java": "java", + "kt": "kotlin", + "swift": "swift", + "c": "c", + "h": "c", + "cpp": "cpp", + "hpp": "cpp", + "cc": "cpp", + "cs": "csharp", + "rb": "ruby", + "sh": "shell", + "bash": "shell", + "zsh": "shell", + "sql": "sql", + "yaml": "yaml", + "yml": "yaml", + "toml": "toml", + "json": "json", + "md": "markdown", + "html": "html", + "css": "css", + "scss": "scss", + "vue": "vue", + "svelte": "svelte", + "php": "php", + "lua": "lua", +} + +// trimAction normalizes a single ceiling/upgrade value for byte-stable +// output: trims surrounding whitespace, strips a single trailing dot or +// comma (artifacts of natural-language authoring), and collapses internal +// runs of whitespace. +func trimAction(s string) string { + s = strings.TrimSpace(s) + for len(s) > 0 { + c := s[len(s)-1] + if c == '.' || c == ',' || c == ';' || c == ':' { + s = s[:len(s)-1] + continue + } + break + } + return strings.Join(strings.Fields(s), " ") +} + +// findColumn computes the 1-based column position of `pattern` in `line`, +// or 0 if not found. The marker scanner uses this purely for diagnostics — +// downstream audit passes can show a precise cursor. +func findColumn(line, pattern string) int { + return strings.Index(line, pattern) + 1 +} + +// nextSymbol looks up at most 40 lines above AND below the marker for a +// Go-style `func Receiver(...)` or a python `def name(` definition. It is +// a best-effort guess — the marker itself does not need to know its +// symbol to be useful, only to be ergonomic when read in a report. The +// `markerLineIdx` argument is a 0-based line index, NOT a byte offset. +// Callers must convert offsets to indices with `byteOffsetOfLine` first +// or — preferred — pass the already-known `Line-1`. +func nextSymbol(content string, markerLineIdx int) string { + if markerLineIdx < 0 { + return "" + } + lines := strings.Split(content, "\n") + if len(lines) == 0 { + return "" + } + startUp := markerLineIdx - 40 + if startUp < 0 { + startUp = 0 + } + startDown := markerLineIdx + 1 + endDown := markerLineIdx + 40 + if endDown > len(lines) { + endDown = len(lines) + } + tryOrder := func(slice []string) string { + for _, line := range slice { + line = strings.TrimSpace(line) + for _, re := range []*regexp.Regexp{ + regexp.MustCompile(`^func\s+(?:\([^)]+\)\s+)?([A-Za-z_][A-Za-z0-9_]*)`), + regexp.MustCompile(`^def\s+([A-Za-z_][A-Za-z0-9_]*)`), + regexp.MustCompile(`^function\s+([A-Za-z_][A-Za-z0-9_]*)`), + regexp.MustCompile(`^class\s+([A-Za-z_][A-Za-z0-9_]*)`), + regexp.MustCompile(`^(?:export\s+)?const\s+([A-Za-z_][A-Za-z0-9_]*)`), + regexp.MustCompile(`^(?:export\s+)?(?:async\s+)?function\s+([A-Za-z_][A-Za-z0-9_]*)`), + } { + if m := re.FindStringSubmatch(line); len(m) == 2 { + return m[1] + } + } + } + return "" + } + // Prefer the closest declaration ABOVE — it represents what the + // marker is documenting. Fall back to the closest declaration + // BELOW when no above-the-line declaration exists (e.g. a marker + // sitting right after a `package` line). + for i := markerLineIdx - 1; i >= startUp; i-- { + if s := tryOrder([]string{lines[i]}); s != "" { + return s + } + } + for i := startDown; i < endDown; i++ { + if s := tryOrder([]string{lines[i]}); s != "" { + return s + } + } + return "" +} + +// ParseFile reads a single file and returns its sorted-by-line markers. +// Returns an empty slice and no error for an unreadable, hidden, or empty +// file; the caller can verify with `len(result) == 0`. +func ParseFile(path string) ([]Marker, error) { + return ParseFileWithCap(path, 0) +} + +// ParseFileWithCap is ParseFile with an explicit per-file byte limit. +// Pass 0 to disable the cap. +func ParseFileWithCap(path string, max int64) ([]Marker, error) { + f, err := os.Open(path) // #nosec G304 — input is a CLI path + if err != nil { + if os.IsNotExist(err) || os.IsPermission(err) { + return nil, nil // ergonomic: missing / unreadable files are empty + } + return nil, fmt.Errorf("sindept: open %s: %w", path, err) + } + defer func() { _ = f.Close() }() + + if max > 0 { + st, err := f.Stat() + if err == nil && st.Size() > max { + return nil, nil + } + } + + // Read fully so the regex can scan multiline content. We bound by + // the caller-supplied `max` above, so this is not unbounded. + var buf bytes.Buffer + if _, err := bufio.NewReader(f).WriteTo(&buf); err != nil { + return nil, fmt.Errorf("sindept: read %s: %w", path, err) + } + content := buf.String() + rawLines := strings.Split(content, "\n") + + idx := markerRe.FindAllStringSubmatchIndex(content, -1) + if len(idx) == 0 { + return nil, nil + } + + out := make([]Marker, 0, len(idx)) + for _, m := range idx { + // [0:1] = full match, [2:3] = reason, [4:5] = upgrade (or -1) + fullStart, fullEnd := m[0], m[1] + reasonS, reasonE := m[2], m[3] + upgS, upgE := m[4], m[5] + + raw := content[fullStart:fullEnd] + reason := trimAction(stripCloser(content[reasonS:reasonE])) + upgrade := "" + hasUpg := false + if upgS >= 0 && upgE >= 0 { + upgrade = trimAction(stripCloser(content[upgS:upgE])) + hasUpg = upgrade != "" + } + + line, col := lineColumn(rawLines, fullStart) + + mark := Marker{ + File: path, + Line: line, + Column: col, + Reason: reason, + Upgrade: upgrade, + HasUpg: hasUpg, + Raw: raw, + Language: extLang[strings.TrimPrefix(strings.ToLower(filepath.Ext(path)), ".")], + Symbol: nextSymbol(content, line-1), + } + out = append(out, mark) + } + return out, nil +} + +// lineColumn maps a byte offset into a line number and 1-based column. +func lineColumn(lines []string, off int) (int, int) { + consumed := 0 + for i, l := range lines { + consumed += len(l) + 1 // +1 for newline + if consumed > off { + col := off - (consumed - len(l) - 1) + 1 + return i + 1, col + } + } + return len(lines), 1 +} + +// byteOffsetOfLine returns the starting byte offset of `idx`-th line in +// `lines` (0-indexed). It is used by ParseFile to look up `Symbol` for +// the marker we just saw. +func byteOffsetOfLine(lines []string, idx int) int { + if idx < 0 { + return 0 + } + if idx >= len(lines) { + return len(strings.Join(lines, "\n")) + } + off := 0 + for i := 0; i < idx; i++ { + off += len(lines[i]) + 1 + } + return off +} + +// ParseDir walks `root` recursively and returns every marker found. +// It is byte-deterministic: file order is lex-sorted, then marker order +// is the order they appear in each file. The result is sorted by File +// then Line then Column. +func ParseDir(root string, opts Options) ([]Marker, error) { + if root == "" { + return nil, fmt.Errorf("sindept: empty root") + } + info, err := os.Stat(root) + if err != nil { + return nil, fmt.Errorf("sindept: stat %s: %w", root, err) + } + if !info.IsDir() { + mk, err := ParseFileWithCap(root, opts.MaxFileBytes) + if err != nil { + return nil, err + } + return mk, nil + } + + skipSet := map[string]bool{} + for _, s := range opts.Skip { + skipSet[s] = true + } + suffixSet := map[string]bool{} + for _, s := range opts.SkipSuffixes { + suffixSet[s] = true + } + extSet := map[string]bool{} + for _, e := range opts.IncludeExt { + extSet[strings.ToLower(strings.TrimPrefix(e, "."))] = true + } + + // The walk is single-threaded so the output is stable regardless of + // goroutine count. sin-debt is a deterministic scan, not a streaming + // search. + var out []Marker + walkErr := filepath.WalkDir(root, func(path string, d fs.DirEntry, err error) error { + if err != nil { + if os.IsNotExist(err) { + return nil + } + return err + } + base := d.Name() + if d.IsDir() { + if shouldSkipDir(base, skipSet) { + return filepath.SkipDir + } + return nil + } + if base == "" || strings.HasPrefix(base, ".") && base != ".env" { + // silently drop hidden files except a small allowance + if base != ".env" { + return nil + } + } + if shouldSkipSuffix(base, suffixSet) { + return nil + } + if strings.HasSuffix(base, ".min.js") || strings.HasSuffix(base, ".min.css") { + return nil + } + ext := strings.TrimPrefix(strings.ToLower(filepath.Ext(base)), ".") + if len(extSet) > 0 && !extSet[ext] { + return nil + } + mks, err := ParseFileWithCap(path, opts.MaxFileBytes) + if err != nil { + return err + } + out = append(out, mks...) + return nil + }) + if walkErr != nil { + return nil, walkErr + } + + sort.Slice(out, func(i, j int) bool { + if out[i].File != out[j].File { + return out[i].File < out[j].File + } + if out[i].Line != out[j].Line { + return out[i].Line < out[j].Line + } + return out[i].Column < out[j].Column + }) + return out, nil +} + +// shouldSkipDir returns true when `base` (a single path component) is in +// `skipSet` or starts with ".". Hidden dirs are skipped unconditionally — +// they should never contain markers we want to scan. +func shouldSkipDir(base string, skipSet map[string]bool) bool { + if base == "" { + return true + } + if strings.HasPrefix(base, ".") { + return true + } + return skipSet[base] +} + +// shouldSkipSuffix returns true when `base` ends with any of the suffixes +// in `suffixSet`. Used to drop test files (`_test.go`) and design docs +// (`.doc.md`) by default — see the rationale on DefaultOptions. +func shouldSkipSuffix(base string, suffixSet map[string]bool) bool { + if len(suffixSet) == 0 { + return false + } + for suf := range suffixSet { + if suf != "" && strings.HasSuffix(base, suf) { + return true + } + } + return false +} diff --git a/cmd/sin-code/internal/sindept/policy.go b/cmd/sin-code/internal/sindept/policy.go new file mode 100644 index 00000000..eca8bbd2 --- /dev/null +++ b/cmd/sin-code/internal/sindept/policy.go @@ -0,0 +1,304 @@ +// SPDX-License-Identifier: MIT +// Purpose: configurable policy for the sin-debt marker convention. The +// policy owns three things: +// +// 1. the canonical default reasons (issue/PR triage templates) +// 2. the optional upgrade trigger catalogue ("properties" detected from +// the marker text) +// 3. the rot-risk thresholds used by `sin-code debt check` so callers +// can plug a tiny `.sin-code/debt-policy.toml` and override defaults +// +// The shape is intentionally TOML-friendly: every field is a primitive +// (string slice / int / bool). No struct cycles, no nested maps. +// Docs: sindept.doc.md +package sindept + +import ( + "bufio" + "fmt" + "os" + "path/filepath" + "strings" +) + +// Policy is the configurable surface for sin-debt. The zero value is +// the conservative default — every field is meaningful on its own, so +// callers can construct a Policy inline (e.g. in tests or CI scripts). +type Policy struct { + // DefaultReasons is the catalogue of ceiling phrases the lint + // recommends authors pick from. Authors can deviate from the list; + // this is only guidance for `debt check --advice`. + DefaultReasons []string + + // UpgradeTriggers catalogues suggested upgrade trigger phrases. The + // value is the canonical phrase; the map key is the slug used in + // `debt stats --by trigger`. + UpgradeTriggers map[string]string + + // MaxNoUpgrade is the soft rot ceiling: above this count, `debt + // check` exits 1 (and CI fails). 0 == disabled. + MaxNoUpgrade int + + // RequireUpgrade forces `debt check` to fail when ANY marker lacks + // the `upgrade:` clause. The default policy disables this — humans + // often write provisional markers — but a CI can opt in by setting + // it true in `.sin-code/debt-policy.toml`. + RequireUpgrade bool + + // Source is the path the Policy was loaded from. Empty if defaults + // only; populated when LoadPolicyFile succeeded. + Source string +} + +// DefaultPolicy is the conservative out-of-the-box policy. It is what the +// `debt` subcommand returns when the user has no `.sin-code/debt-policy.toml`. +// +// The DefaultReasons / UpgradeTriggers lists mirror ponytail's tagging +// conventions (delete / stdlib / native / yagni / shrink) so existing +// adopters have a familiar starting point. +func DefaultPolicy() Policy { + return Policy{ + DefaultReasons: []string{ + "global mutex", + "O(n²) scan", + "hand-rolled retry", + "hand-rolled backoff", + "in-memory cache", + "in-process queue", + "tied loop", + "manual json encode", + "manual json decode", + "disabled keepalive", + "single-threaded worker", + "polling", + "polling-only watcher", + "synchronous I/O", + "synchronous fan-out", + "blocking sleep", + "rebuild-on-every-call", + "reload-on-import", + "polling timer ticks", + "best-effort retry", + "best-effort dedup", + }, + UpgradeTriggers: map[string]string{ + "throughput": "when throughput exceeds threshold", + "scale": "when N exceeds threshold", + "latency": "when latency exceeds threshold", + "errors": "when error rate is non-trivial", + "main": "when the upstream API stabilises", + "stable": "when the upstream API stabilises", + "context": "when context cancellation is required", + "rswitch": "switch to when threshold breached", + }, + MaxNoUpgrade: 50, + RequireUpgrade: false, + } +} + +// LoadPolicyFile overlays the on-disk `path` onto DefaultPolicy(). The +// file is a tiny subset of TOML: +// +// [sin-debt] +// max_no_upgrade = 50 +// require_upgrade = false +// default_reasons = ["global mutex", "O(n²) scan"] +// [sin-debt.upgrade_triggers] +// throughput = "when throughput exceeds threshold" +// main = "when the upstream API stabilises" +// +// Unknown keys are tolerated (forward-compat). A missing file returns the +// default policy with Source="". A malformed file returns an error, +// because the user explicitly asked for it. +func LoadPolicyFile(path string) (Policy, error) { + policy := DefaultPolicy() + if path == "" { + return policy, nil + } + f, err := os.Open(path) // #nosec G304 — loaded by user + if err != nil { + if os.IsNotExist(err) { + return policy, nil + } + return policy, fmt.Errorf("sindept: open policy %s: %w", path, err) + } + defer func() { _ = f.Close() }() + + section := "" + triggers := map[string]string{} + var sc = bufio.NewScanner(f) + for sc.Scan() { + line := strings.TrimSpace(sc.Text()) + if line == "" || strings.HasPrefix(line, "#") { + continue + } + if strings.HasPrefix(line, "[") && strings.HasSuffix(line, "]") { + section = strings.TrimSpace(line[1 : len(line)-1]) + continue + } + eq := strings.IndexByte(line, '=') + if eq <= 0 { + continue + } + key := strings.TrimSpace(line[:eq]) + val := strings.TrimSpace(line[eq+1:]) + val = strings.Trim(val, `"'`) + + switch section { + case "sin-debt": + switch key { + case "max_no_upgrade": + if n, err := parseInt(val); err == nil { + policy.MaxNoUpgrade = n + } + case "require_upgrade": + policy.RequireUpgrade = parseBool(val) + case "default_reasons": + policy.DefaultReasons = parseStringList(val) + } + case "sin-debt.upgrade_triggers": + if key != "" && val != "" { + triggers[key] = val + } + } + } + if err := sc.Err(); err != nil { + return policy, fmt.Errorf("sindept: scan policy %s: %w", path, err) + } + if len(triggers) > 0 { + policy.UpgradeTriggers = triggers + } + policy.Source = path + return policy, nil +} + +// LoadPolicyForRoot walks up from `root` looking for `.sin-code/debt-policy.toml`. +// Stops at the first hit or the filesystem root. The returned policy.Source +// is the absolute path that matched. +func LoadPolicyForRoot(root string) (Policy, error) { + abs, err := filepath.Abs(root) + if err != nil { + return DefaultPolicy(), nil + } + dir := abs + for { + candidate := filepath.Join(dir, ".sin-code", "debt-policy.toml") + if _, err := os.Stat(candidate); err == nil { + return LoadPolicyFile(candidate) + } + parent := filepath.Dir(dir) + if parent == dir { + return DefaultPolicy(), nil + } + dir = parent + } +} + +// CheckResult is what `debt check` prints and exits on. +type CheckResult struct { + Ok bool + Total int + Missing int + Threshold int + Failed []Marker + RequireUpg bool + MissingUpg []Marker +} + +// RunCheck implements the `debt check` policy gate. It exits non-zero +// (CheckResult.Ok == false) when either: +// +// 1. The count of markers without an `upgrade:` clause exceeds +// policy.MaxNoUpgrade (when MaxNoUpgrade > 0), OR +// 2. Policy.RequireUpgrade is true and any marker lacks `upgrade:`. +// +// Failed is populated with the markers that tripped the gate so the +// human can fix them. When MaxNoUpgrade trips, Failed is capped at the +// offending population (so the printed list is bounded). +func (p Policy) RunCheck(markers []Marker) CheckResult { + res := CheckResult{Threshold: p.MaxNoUpgrade, RequireUpg: p.RequireUpgrade, Total: len(markers)} + for _, m := range markers { + if !(m.HasUpg && m.Upgrade != "") { + res.Missing++ + res.MissingUpg = append(res.MissingUpg, m) + } + } + if p.MaxNoUpgrade > 0 && res.Missing > p.MaxNoUpgrade { + res.Failed = append(res.Failed, res.MissingUpg...) + res.Ok = false + return res + } + if p.RequireUpgrade && res.Missing > 0 { + res.Failed = append(res.Failed, res.MissingUpg...) + res.Ok = false + return res + } + res.Ok = true + return res +} + +// FormatCheckResult renders the gate verdict as a markdown bullet list. +// Used by `debt check` and by tests as golden output. +func FormatCheckResult(r CheckResult) string { + var b strings.Builder + if r.Ok { + fmt.Fprintf(&b, "- ok: %d markers, %d missing upgrade, threshold=%d\n", + r.Total, r.Missing, r.Threshold) + return b.String() + } + fmt.Fprintf(&b, "- FAIL: %d markers, %d missing upgrade, threshold=%d\n", + r.Total, r.Missing, r.Threshold) + if r.RequireUpg { + fmt.Fprintln(&b, "- reason: require_upgrade is true and at least one marker has no upgrade clause") + } + if r.Threshold > 0 && r.Missing > r.Threshold { + fmt.Fprintf(&b, "- reason: missing-upgrade count %d exceeds threshold %d\n", r.Missing, r.Threshold) + } + for _, m := range r.Failed { + fmt.Fprintf(&b, " - %s:%d — %s\n", m.File, m.Line, m.Reason) + } + return b.String() +} + +// parseInt is a tiny strconv substitute that ignores trailing chars. +func parseInt(s string) (int, error) { + var n int + for _, r := range s { + if r < '0' || r > '9' { + break + } + n = n*10 + int(r-'0') + } + if n == 0 && s != "0" { + return 0, fmt.Errorf("not a number: %q", s) + } + return n, nil +} + +func parseBool(s string) bool { + switch strings.ToLower(strings.TrimSpace(s)) { + case "true", "yes", "on", "1": + return true + } + return false +} + +// parseStringList accepts `["a", "b"]`, `["a","b"]` and the simpler +// `a, b, c` form. The result is trimmed and dropped if empty. +func parseStringList(s string) []string { + s = strings.TrimSpace(s) + s = strings.TrimPrefix(s, "[") + s = strings.TrimSuffix(s, "]") + if s == "" { + return nil + } + var out []string + for _, raw := range strings.Split(s, ",") { + v := strings.TrimSpace(raw) + v = strings.Trim(v, `"'`) + if v != "" { + out = append(out, v) + } + } + return out +} diff --git a/cmd/sin-code/internal/sindept/report.go b/cmd/sin-code/internal/sindept/report.go new file mode 100644 index 00000000..32b99540 --- /dev/null +++ b/cmd/sin-code/internal/sindept/report.go @@ -0,0 +1,205 @@ +// SPDX-License-Identifier: MIT +// Purpose: byte-deterministic markdown report renderer for sin-dept marker +// sets. Render produces the same bytes for two Stats that aggregate the +// same marker set, regardless of the order in which files were walked or +// goroutines executed. +// +// Docs: sindept.doc.md +package sindept + +import ( + "fmt" + "io" + "strconv" + "strings" +) + +// ReportSection enumerates the optional sections Render* funcs can emit. +// They are pre-sorted (lexicographic) so the output is deterministic. +type ReportSection string + +const ( + SectionSummary ReportSection = "summary" + SectionRotRisk ReportSection = "rot_risk" + SectionByFile ReportSection = "by_file" + SectionByReason ReportSection = "by_reason" + SectionByLang ReportSection = "by_lang" + SectionBySymbol ReportSection = "by_symbol" + SectionMarkers ReportSection = "markers" +) + +// DefaultSections is the lex-ordered set emitted by Render and used by +// `sin-code debt stats`. Hard-coded so the per-file report and the +// aggregate dashboard stay byte-identical for the same input. +func DefaultSections() []ReportSection { + return []ReportSection{ + SectionByFile, + SectionByLang, + SectionByReason, + SectionRotRisk, + SectionSummary, + } +} + +// FormatVersion is the canonical string embedded in every rendered report. +// Bumping it (and updating tests) is on the table when the schema changes. +const FormatVersion = "sin-debt/v1" + +// Header is the single line printed at the top of every report. It is +// kept on one line so `rg "sin-debt/v1"` reliably finds a report even +// when it is concatenated with PR noise. +func Header() string { + return fmt.Sprintf("# sin-debt report (%s)\n", FormatVersion) +} + +// RenderStats writes the canonical markdown report for `s` to `w`. The +// output is byte-deterministic: aggregates were already sorted by +// AggregateStats; rendering prints them in a fixed order; floats are +// formatted with a fixed precision. +func RenderStats(w io.Writer, s Stats, sections []ReportSection) { + if sections == nil { + sections = DefaultSections() + } + fmt.Fprint(w, Header()) + + for _, sec := range sections { + switch sec { + case SectionSummary: + renderSummary(w, s) + case SectionRotRisk: + renderRotRisk(w, s) + case SectionByFile: + renderKVTable(w, "By file", s.ByFile, s.Total) + case SectionByReason: + renderKVTable(w, "By reason", s.ByReason, s.Total) + case SectionByLang: + renderKVTable(w, "By language", s.ByLanguage, s.Total) + case SectionBySymbol: + renderKVTable(w, "By symbol", s.BySymbol, s.Total) + } + } +} + +// RenderStatsString is the stringly-typed wrapper used by tests and the +// `debt stats` subcommand. Equivalent to RenderStats(&buf, s, nil). +func RenderStatsString(s Stats) string { + var b strings.Builder + RenderStats(&b, s, nil) + return b.String() +} + +// RenderListString writes a marker list as a markdown table — one row +// per marker. The marker slice must already be sorted by File/Line/Column +// (ParseDir guarantees this) for the report to be deterministic. +func RenderListString(markers []Marker) string { + var b strings.Builder + fmt.Fprint(&b, Header()) + fmt.Fprintln(&b, "## Markers") + fmt.Fprintln(&b) + fmt.Fprintln(&b, "| file | line | symbol | reason | upgrade | rot |") + fmt.Fprintln(&b, "|------|------|--------|--------|---------|-----|") + for _, m := range markers { + fmt.Fprintf(&b, "| %s | %d | %s | %s | %s | %s |\n", + escapeCell(m.File), + m.Line, + escapeCell(m.Symbol), + escapeCell(m.Reason), + escapeCell(m.Upgrade), + rotCell(m), + ) + } + fmt.Fprintln(&b) + fmt.Fprintf(&b, "_%d markers total. %d with upgrade, %d in rot-risk._\n", + len(markers), countWithUpgrade(markers), countNoUpgrade(markers)) + return b.String() +} + +func renderSummary(w io.Writer, s Stats) { + fmt.Fprintln(w, "## Summary") + fmt.Fprintln(w) + fmt.Fprintf(w, "- **Total markers:** %d\n", s.Total) + fmt.Fprintf(w, "- **With upgrade:** %d\n", s.WithUpgrade) + fmt.Fprintf(w, "- **Without upgrade (rot-risk):** %d\n", s.WithoutUpgrade) + fmt.Fprintf(w, "- **Files scanned:** %d\n", s.FilesScanned) + fmt.Fprintf(w, "- **Markers per file:** %s\n", formatFloat(s.MarkersPerFile)) + if s.Oldest != nil { + fmt.Fprintf(w, "- **Oldest marker:** %s:%d (%s)\n", + s.Oldest.File, s.Oldest.Line, s.Oldest.Reason) + } + fmt.Fprintln(w) +} + +func renderRotRisk(w io.Writer, s Stats) { + if s.WithoutUpgrade == 0 { + return + } + fmt.Fprintln(w, "## Rot-risk markers (no upgrade clause)") + fmt.Fprintln(w) + fmt.Fprintln(w, "| file | line | symbol | reason |") + fmt.Fprintln(w, "|------|------|--------|--------|") + for _, m := range s.RotRisk { + fmt.Fprintf(w, "| %s | %d | %s | %s |\n", + escapeCell(m.File), m.Line, + escapeCell(m.Symbol), escapeCell(m.Reason)) + } + fmt.Fprintln(w) +} + +func renderKVTable(w io.Writer, title string, rows []KV, total int) { + if len(rows) == 0 { + return + } + fmt.Fprintf(w, "## %s\n\n", title) + fmt.Fprintln(w, "| key | count | share |") + fmt.Fprintln(w, "|-----|-------|-------|") + for _, kv := range rows { + share := 0.0 + if total > 0 { + share = float64(kv.Count) / float64(total) * 100 + } + fmt.Fprintf(w, "| %s | %d | %s%% |\n", + escapeCell(kv.Key), kv.Count, formatFloat(share)) + } + fmt.Fprintln(w) +} + +// formatFloat keeps precision fixed at one decimal so two reports over +// the same Stats cannot drift by sub-cent randoms in floating-point math. +func formatFloat(v float64) string { + return strconv.FormatFloat(v, 'f', 1, 64) +} + +// escapeCell collapses newlines/tabs/pipes so markdown table cells stay +// well-formed. Deterministic: every rune that would break the cell is +// replaced with a literal space. +func escapeCell(s string) string { + if s == "" { + return "" + } + s = strings.ReplaceAll(s, "\n", " ") + s = strings.ReplaceAll(s, "\r", " ") + s = strings.ReplaceAll(s, "\t", " ") + s = strings.ReplaceAll(s, "|", "/") + return strings.TrimSpace(s) +} + +func rotCell(m Marker) string { + if m.HasUpg && m.Upgrade != "" { + return "ok" + } + return "rot" +} + +func countWithUpgrade(ms []Marker) int { + n := 0 + for _, m := range ms { + if m.HasUpg && m.Upgrade != "" { + n++ + } + } + return n +} + +func countNoUpgrade(ms []Marker) int { + return len(ms) - countWithUpgrade(ms) +} diff --git a/cmd/sin-code/internal/sindept/sindept.doc.md b/cmd/sin-code/internal/sindept/sindept.doc.md new file mode 100644 index 00000000..3813a445 --- /dev/null +++ b/cmd/sin-code/internal/sindept/sindept.doc.md @@ -0,0 +1,111 @@ +# sin-dept Marker Convention (`internal/sindept`) + +Docs: `parser.go`, `stats.go`, `report.go`, `policy.go` + +## What + +A first-class, parseable convention for marking **intentional** complexity +in source code. Each marker names the ceiling of the current shortcut and +the trigger to revisit it. The marker scanner (`internal/sindept`) is +byte-stable, language-agnostic, and feeds two downstream consumers: + +| Consumer | Issue | Role | +|----------|-------|------| +| `sin-code debt list / stats / check` | #177 (this package) | Human-readable reports + CI gate | +| complexity auditor | #179 | Auto-tagging related complexity findings as "approved shortcut" | +| audit-engine | #180 | Aggregate one-shot boolean of "do you have rot?" | + +## Format + +``` +// sin-debt: , upgrade: +// sin-debt: // upgrade is OPTIONAL but RECOMMENDED +``` + +- **Comment families** recognised out of the box: `//`, `#`, `--`, `/* … */`, + ``. The opener and any trailing closer are stripped silently + from the captured values. +- **Reason / Upgrade** values are trimmed, surrounding punctuation stripped, + internal whitespace collapsed — so two markers authored with different + whitespace produce the same `Marker.Reason` byte-for-byte. +- **Upgrade clause is optional** — markers without one are tagged + `rot-risk` in the report and trigger a non-zero exit on + `sin-code debt check` when policy acceptance is configured. + +### Examples from real source code + +```go +// sin-debt: global mutex, upgrade: per-account locks when throughput > 1k req/s +// sin-debt: O(n²) scan, upgrade: switch to map lookup when n > 100 +// sin-debt: hand-rolled retry, upgrade: use cenkalti/backoff when context cancellation matters +// sin-debt: this exists +``` + +```python +# sin-debt: polling timer ticks, upgrade: switch to fsnotify when file count > 100 +``` + +```markdown + +``` + +## Files + +| Path | Purpose | +|------|---------| +| `parser.go` | `Marker`, regex, `ParseFile`, `ParseDir` | +| `stats.go` | `Stats` aggregator, `ByFile/ByReason/ByLanguage/BySymbol` views | +| `report.go` | byte-deterministic `Render*` markdown functions | +| `policy.go` | configurable default reasons + upgrade triggers + rot thresholds | + +## Usage + +```go +import "github.com/OpenSIN-Code/SIN-Code/cmd/sin-code/internal/sindept" + +mk, err := sindept.ParseFile("cmd/sin-code/internal/foo/foo.go") +stats := sindept.AggregateStats(mk) +fmt.Println(sindept.RenderStatsString(stats)) +``` + +```bash +# CLI surface +sin-code debt list --path . # markdown table of every marker +sin-code debt stats --by reason # report grouped by reason +sin-code debt stats --by file # report grouped by file +sin-code debt stats --by age # chronological: oldest first +sin-code debt check # CI gate, exits 1 when rot > threshold +``` + +## Policy file (`.sin-code/debt-policy.toml`) + +```toml +[sin-debt] +max_no_upgrade = 50 # soft ceiling; above this `debt check` fails +require_upgrade = false # when true, ANY marker without upgrade fails + +[sin-debt.upgrade_triggers] +throughput = "when throughput exceeds threshold" # suggest keyword `throughput` as a known trigger +main = "when the upstream API stabilises" +``` + +The walk looks up the policy from the scan root upwards — the closest +`.sin-code/debt-policy.toml` wins. A missing file = default policy. + +## Byte-stability promise + +`RenderStatsString` and `RenderListString` are byte-deterministic for the +same Stats value. Two scans of the same tree (different process, same +machine) MUST emit the same bytes. The `FormatVersion` const +(`sin-debt/v1`) is embedded so tests can pin the format and reviewers +can `rg "sin-debt/v1"` to find a generated report. + +## Maintenance + +- When adding a new comment family (e.g. `%` for Erlang), extend + `markerRe` in parser.go and add a parse test in sindept_test.go. +- Renaming a `Marker` field is a breaking change; bump the format + version and add a deprecation alias on the JSON tag. +- The `DefaultReasons` and `UpgradeTriggers` lists mirror ponytail's + recommended tags so existing adopters have a familiar starting point. + Add new entries alphabetically; never delete existing ones. diff --git a/cmd/sin-code/internal/sindept/sindept_test.go b/cmd/sin-code/internal/sindept/sindept_test.go new file mode 100644 index 00000000..fa002796 --- /dev/null +++ b/cmd/sin-code/internal/sindept/sindept_test.go @@ -0,0 +1,390 @@ +// SPDX-License-Identifier: MIT +// Purpose: tests for the sindept package (issue #177). The golden fixture +// set is 10 markers spread across five files (Go, Python, TypeScript, +// Shell, Markdown). Tests assert: +// +// 1. marker count (10) +// 2. parser recognises every comment family (`//`, `#`, `--`, `/*`, ``) from the +// captured reason / upgrade clauses +// 4. AggregateStats byte-stable view matches the documented golden +// 5. RenderStatsString bytes are deterministic for the same Stats +// 6. Policy.RunCheck gate semantics (above-threshold fails, below passes) +// 7. Wire bytes survive RoundTrip (Render -> sort -> split -> stable) +package sindept + +import ( + "os" + "path/filepath" + "sort" + "strings" + "testing" +) + +func TestParseGoldenTotalCount(t *testing.T) { + mk := parseGolden(t) + if len(mk) != 10 { + t.Fatalf("expected 10 golden markers, got %d", len(mk)) + } +} + +func TestParseGoldenFiles(t *testing.T) { + mk := parseGolden(t) + wantFiles := []string{ + "testdata/markers.go", + "testdata/markers.md", + "testdata/markers.py", + "testdata/markers.sh", + "testdata/markers.ts", + } + gotFiles := uniqueSortedFiles(mk) + if !equalSlices(gotFiles, wantFiles) { + t.Fatalf("file set mismatch: got %v want %v", gotFiles, wantFiles) + } +} + +func TestParseGoldenFamilyCoverage(t *testing.T) { + mk := parseGolden(t) + wantFamilies := map[string]bool{ + "//": false, "#": false, "") { + t.Errorf("file=%s reason=%q still has trailing closer", m.File, m.Reason) + } + if strings.HasSuffix(m.Upgrade, "*/") || strings.HasSuffix(m.Upgrade, "-->") { + t.Errorf("file=%s upgrade=%q still has trailing closer", m.File, m.Upgrade) + } + } +} + +func TestAggregateStatsMatchesGoldenView(t *testing.T) { + mk := parseGolden(t) + stats := AggregateStats(mk) + if stats.Total != 10 { + t.Fatalf("Total=%d want 10", stats.Total) + } + if stats.WithUpgrade != 6 { + t.Fatalf("WithUpgrade=%d want 6", stats.WithUpgrade) + } + if stats.WithoutUpgrade != 4 { + t.Fatalf("WithoutUpgrade=%d want 4", stats.WithoutUpgrade) + } + if len(stats.ByFile) != 5 { + t.Fatalf("ByFile size=%d want 5", len(stats.ByFile)) + } + if len(stats.ByLanguage) != 5 { + t.Fatalf("ByLanguage size=%d want 5", len(stats.ByLanguage)) + } + if len(stats.RotRisk) != 4 { + t.Fatalf("RotRisk size=%d want 4", len(stats.RotRisk)) + } +} + +func TestRenderStatsByteStable(t *testing.T) { + mk := parseGolden(t) + s1 := RenderStatsString(AggregateStats(mk)) + s2 := RenderStatsString(AggregateStats(mk)) + if s1 != s2 { + t.Fatalf("RenderStatsString not byte-stable:\n---1---\n%s\n---2---\n%s", s1, s2) + } + if !strings.HasPrefix(s1, Header()) { + t.Fatalf("missing header in rendered report:\n%s", s1) + } + for _, want := range []string{ + "## Summary", + "## By file", + "## By reason", + "## By language", + "## Rot-risk markers", + "Total markers:", + } { + if !strings.Contains(s1, want) { + t.Errorf("rendered report missing %q", want) + } + } +} + +func TestRenderListStringByteStable(t *testing.T) { + mk := parseGolden(t) + a := RenderListString(mk) + b := RenderListString(mk) + if a != b { + t.Fatalf("RenderListString not byte-stable") + } + if !strings.Contains(a, "| file | line | symbol | reason | upgrade | rot |") { + t.Fatalf("missing header row in list report") + } + if !strings.Contains(a, "_10 markers total. 6 with upgrade, 4 in rot-risk._") { + t.Fatalf("missing footer counts in list report") + } +} + +func TestParseFileReturnsEmptyForMissingPath(t *testing.T) { + mk, err := ParseFile("/this/path/does/not/exist.go") + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if mk != nil { + t.Fatalf("expected nil, got %v", mk) + } +} + +func TestParseDirSkipsVendoredAndHidden(t *testing.T) { + dir := t.TempDir() + writeFile(t, filepath.Join(dir, "real.go"), `// sin-debt: ok +package x +`) + gitDir := filepath.Join(dir, ".git") + if err := os.Mkdir(gitDir, 0o755); err != nil { + t.Fatal(err) + } + writeFile(t, filepath.Join(gitDir, "HEAD"), `// sin-debt: skip me +`) + vendorDir := filepath.Join(dir, "vendor") + if err := os.Mkdir(vendorDir, 0o755); err != nil { + t.Fatal(err) + } + writeFile(t, filepath.Join(vendorDir, "x.go"), `// sin-debt: skip me +`) + nmDir := filepath.Join(dir, "node_modules") + if err := os.Mkdir(nmDir, 0o755); err != nil { + t.Fatal(err) + } + writeFile(t, filepath.Join(nmDir, "x.js"), `// sin-debt: skip me +`) + mk, err := ParseDir(dir, DefaultOptions()) + if err != nil { + t.Fatalf("ParseDir: %v", err) + } + if len(mk) != 1 { + t.Fatalf("expected 1 marker, got %d", len(mk)) + } + if !strings.HasSuffix(mk[0].File, "real.go") { + t.Fatalf("expected real.go, got %s", mk[0].File) + } +} + +func TestParseDirSortIsDeterministic(t *testing.T) { + mk := parseGolden(t) + for i := 1; i < len(mk); i++ { + if mk[i].File < mk[i-1].File { + t.Fatalf("not sorted by File at index %d", i) + } + if mk[i].File == mk[i-1].File && mk[i].Line < mk[i-1].Line { + t.Fatalf("not sorted by Line within file at index %d", i) + } + } +} + +func TestPolicyCheckPassesWhenUnderThreshold(t *testing.T) { + p := DefaultPolicy() + p.MaxNoUpgrade = 5 + p.RequireUpgrade = false + mk := parseGolden(t) + r := p.RunCheck(mk) + if !r.Ok { + t.Fatalf("expected ok, got %+v", r) + } +} + +func TestPolicyCheckFailsOverThreshold(t *testing.T) { + p := DefaultPolicy() + p.MaxNoUpgrade = 1 + p.RequireUpgrade = false + mk := parseGolden(t) + r := p.RunCheck(mk) + if r.Ok { + t.Fatalf("expected fail, got %+v", r) + } + if len(r.Failed) != 4 { + t.Fatalf("expected 4 failed markers, got %d", len(r.Failed)) + } +} + +func TestPolicyRequireUpgradeForceFails(t *testing.T) { + p := DefaultPolicy() + p.RequireUpgrade = true + p.MaxNoUpgrade = 0 + mk := parseGolden(t) + r := p.RunCheck(mk) + if r.Ok { + t.Fatalf("expected fail with require_upgrade=true") + } + if len(r.Failed) != 4 { + t.Fatalf("expected 4 failed markers, got %d", len(r.Failed)) + } +} + +func TestParseInlineCommentFamilies(t *testing.T) { + cases := []struct { + name string + src string + want int + }{ + {"go", "// sin-debt: go fixture\n", 1}, + {"python", "# sin-debt: py fixture\n", 1}, + {"shell", "# sin-debt: sh fixture\n", 1}, + {"rust", "// sin-debt: rs fixture\n", 1}, + {"c-block", "/* sin-debt: c fixture */\n", 1}, + {"html", "\n", 1}, + {"yaml", "# sin-debt: yaml fixture\n", 1}, + {"multiple", "// sin-debt: one\n// sin-debt: two, upgrade: never\n", 2}, + {"empty", "no markers here\n", 0}, + } + for _, c := range cases { + t.Run(c.name, func(t *testing.T) { + dir := t.TempDir() + path := filepath.Join(dir, "src."+c.name) + if err := writeFileSafe(path, c.src); err != nil { + t.Fatal(err) + } + mk, err := ParseFile(path) + if err != nil { + t.Fatalf("ParseFile: %v", err) + } + if len(mk) != c.want { + t.Fatalf("got %d markers, want %d (%v)", len(mk), c.want, mk) + } + }) + } +} + +func TestParseIgnoresPlainTextMention(t *testing.T) { + dir := t.TempDir() + path := filepath.Join(dir, "doc.go") + src := "// This package documents the `sin-debt:` convention in prose.\n// It explains how to write markers but contains no actual marker.\n" + if err := writeFileSafe(path, src); err != nil { + t.Fatal(err) + } + mk, err := ParseFile(path) + if err != nil { + t.Fatalf("ParseFile: %v", err) + } + if len(mk) != 0 { + t.Fatalf("expected 0 markers (text match should not be a marker), got %+v", mk) + } +} + +func TestParseMaxFileBytesSkipsLargeFiles(t *testing.T) { + dir := t.TempDir() + path := filepath.Join(dir, "huge.go") + huge := strings.Repeat("// sin-debt: nope\n", 10000) + if err := writeFileSafe(path, huge); err != nil { + t.Fatal(err) + } + mk, err := ParseFileWithCap(path, 1000) + if err != nil { + t.Fatalf("ParseFileWithCap: %v", err) + } + if mk != nil { + t.Fatalf("expected nil for over-cap file, got %v", mk) + } +} + +// parseGolden loads every fixture file from `testdata/` and returns a +// single sorted slice. The fixtures are committed so the test is +// idempotent across runs. +func parseGolden(t *testing.T) []Marker { + t.Helper() + files := []string{ + "testdata/markers.go", + "testdata/markers.py", + "testdata/markers.ts", + "testdata/markers.sh", + "testdata/markers.md", + } + var all []Marker + for _, f := range files { + mk, err := ParseFile(f) + if err != nil { + t.Fatalf("ParseFile(%s): %v", f, err) + } + all = append(all, mk...) + } + sort.Slice(all, func(i, j int) bool { + if all[i].File != all[j].File { + return all[i].File < all[j].File + } + return all[i].Line < all[j].Line + }) + return all +} + +func uniqueSortedFiles(mk []Marker) []string { + set := map[string]bool{} + for _, m := range mk { + set[m.File] = true + } + out := make([]string, 0, len(set)) + for k := range set { + out = append(out, k) + } + sort.Strings(out) + return out +} + +func equalSlices(a, b []string) bool { + if len(a) != len(b) { + return false + } + for i := range a { + if a[i] != b[i] { + return false + } + } + return true +} + +func writeFile(t *testing.T, path, content string) { + t.Helper() + if err := writeFileSafe(path, content); err != nil { + t.Fatal(err) + } +} + +func writeFileSafe(path, content string) error { + return os.WriteFile(path, []byte(content), 0o644) +} diff --git a/cmd/sin-code/internal/sindept/stats.go b/cmd/sin-code/internal/sindept/stats.go new file mode 100644 index 00000000..ce282ffe --- /dev/null +++ b/cmd/sin-code/internal/sindept/stats.go @@ -0,0 +1,147 @@ +// SPDX-License-Identifier: MIT +// Purpose: deterministic aggregate stats over a slice of sin-dept markers. +// The Stats struct is intentionally a value type with sorted map slices; the +// downstream renderer (report.go) is byte-stable exactly because every +// aggregation step sorts its keys before producing output. +// Docs: sindept.doc.md +package sindept + +import ( + "sort" + "strings" +) + +// Stats is the aggregate view of a marker set. It is the unit that the +// `sin-code debt stats` subcommand and the `report.go` markdown renderer +// operate on. Every map is materialized as `[]KV` for byte-determinism. +type Stats struct { + // Total is the marker count (with and without upgrade). + Total int + // WithUpgrade counts markers that name an explicit upgrade clause. + WithUpgrade int + // WithoutUpgrade counts markers that do not — these are rot-risk + // ("no-trigger") entries. + WithoutUpgrade int + + // ByFile groups marker counts by absolute file path. + ByFile []KV + // ByReason groups counts by the free-text reason (trimmed, lowercased). + ByReason []KV + // ByLanguage groups counts by detected language tag. + ByLanguage []KV + // BySymbol groups counts by the best-effort symbol guess. + BySymbol []KV + + // RotRisk lists the markers that have no upgrade clause — the rows + // humans should look at first. It is pre-sorted by File then Line. + RotRisk []Marker + + // Oldest is the marker with the smallest Line number (file path + // tie-broken by sort). It is nil when the input is empty. + Oldest *Marker + + // FilesScanned counts files visited during the walk (including files + // with no markers). It is informational only — not part of the + // rot-risk calculation. + FilesScanned int + // MarkersPerFile is Total / max(1, len(ByFile)). + MarkersPerFile float64 +} + +// KV is a lexicographically-stable key+count pair. +type KV struct { + Key string + Count int +} + +// AggregateStats turns a slice of markers into a stable Stats value. The +// input slice can be in any order; everything downstream sorts by key. +// +// The contract is byte-stable: two calls with the same marker set produce +// identical Stats (and therefore identical Render output). +func AggregateStats(markers []Marker) Stats { + s := Stats{Total: len(markers)} + if len(markers) == 0 { + return s + } + + files := make(map[string]int) + reasons := make(map[string]int) + langs := make(map[string]int) + symbols := make(map[string]int) + + for i := range markers { + m := &markers[i] + if m.HasUpg && m.Upgrade != "" { + s.WithUpgrade++ + } else { + s.WithoutUpgrade++ + s.RotRisk = append(s.RotRisk, *m) + } + files[m.File]++ + reasons[strings.ToLower(strings.TrimSpace(m.Reason))]++ + if m.Language != "" { + langs[m.Language]++ + } + if m.Symbol != "" { + symbols[m.Symbol]++ + } + } + + s.ByFile = sortedKV(files) + s.MarkersPerFile = float64(s.Total) / float64(maxInt(1, len(s.ByFile))) + s.ByReason = sortedKV(reasons) + s.ByLanguage = sortedKV(langs) + s.BySymbol = sortedKV(symbols) + s.FilesScanned = len(s.ByFile) + + // Oldest = the lex-smallest (File, Line) marker. + oldest := &markers[0] + for i := range markers { + if markers[i].File < oldest.File || + (markers[i].File == oldest.File && markers[i].Line < oldest.Line) { + oldest = &markers[i] + } + } + s.Oldest = oldest + + // Sort rot-risk lex-by (File, Line, Column). + sort.Slice(s.RotRisk, func(i, j int) bool { + if s.RotRisk[i].File != s.RotRisk[j].File { + return s.RotRisk[i].File < s.RotRisk[j].File + } + if s.RotRisk[i].Line != s.RotRisk[j].Line { + return s.RotRisk[i].Line < s.RotRisk[j].Line + } + return s.RotRisk[i].Column < s.RotRisk[j].Column + }) + return s +} + +// sortedKV converts a map into the lex-sorted []KV view. +// +// Two input maps with the same (key, count) pairs always emit the same +// []KV. The deterministic sort order keeps Render output stable. +func sortedKV(m map[string]int) []KV { + out := make([]KV, 0, len(m)) + for k, v := range m { + out = append(out, KV{Key: k, Count: v}) + } + sort.Slice(out, func(i, j int) bool { + if out[i].Count != out[j].Count { + return out[i].Count > out[j].Count + } + return out[i].Key < out[j].Key + }) + return out +} + +// maxInt returns the larger of a and b. It is a local helper rather than +// the built-in `max` because that helper is Go 1.21+ only and we keep the +// package runnable on the minimum Go version the repo declares. +func maxInt(a, b int) int { + if a > b { + return a + } + return b +} diff --git a/cmd/sin-code/internal/sindept/testdata/markers.go b/cmd/sin-code/internal/sindept/testdata/markers.go new file mode 100644 index 00000000..99043805 --- /dev/null +++ b/cmd/sin-code/internal/sindept/testdata/markers.go @@ -0,0 +1,18 @@ +// SPDX-License-Identifier: MIT +// Purpose: real golden-file fixtures for the sindept package scanner. +// The marker comments below are the source of truth - any reflow must +// preserve the exact byte offsets that TestParseGolden asserts. +package fixtures + +// sin-debt: global mutex, upgrade: per-account locks when throughput > 1k req/s +// sin-debt: O(n²) scan, upgrade: switch to map lookup when n > 100 +// sin-debt: hand-rolled retry, upgrade: use cenkalti/backoff when context cancellation matters +// sin-debt: this exists +func FixturesAreAtTopOfFile() {} + +const ( + FixtureLineA = 9 + FixtureLineB = 10 + FixtureLineC = 11 + FixtureLineD = 12 +) diff --git a/cmd/sin-code/internal/sindept/testdata/markers.md b/cmd/sin-code/internal/sindept/testdata/markers.md new file mode 100644 index 00000000..66b110af --- /dev/null +++ b/cmd/sin-code/internal/sindept/testdata/markers.md @@ -0,0 +1,5 @@ + + + + + diff --git a/cmd/sin-code/internal/sindept/testdata/markers.py b/cmd/sin-code/internal/sindept/testdata/markers.py new file mode 100644 index 00000000..1f5f6cf8 --- /dev/null +++ b/cmd/sin-code/internal/sindept/testdata/markers.py @@ -0,0 +1,8 @@ +# SPDX-License-Identifier: MIT +# Purpose: golden-file fixtures for sindept_test.go (python flavour). +# Do not reflow. + +# sin-debt: polling timer ticks, upgrade: switch to fsnotify when file count > 100 +# sin-debt: synchronous I/O +def fixtures_at_top_of_file(): + pass diff --git a/cmd/sin-code/internal/sindept/testdata/markers.sh b/cmd/sin-code/internal/sindept/testdata/markers.sh new file mode 100644 index 00000000..53be48e3 --- /dev/null +++ b/cmd/sin-code/internal/sindept/testdata/markers.sh @@ -0,0 +1,9 @@ +#!/usr/bin/env bash +# SPDX-License-Identifier: MIT +# Purpose: golden-file fixtures for sindept_test.go (shell flavour). +# Do not reflow. + +# sin-debt: polling +fixtures_at_top_of_file() { + return 0 +} diff --git a/cmd/sin-code/internal/sindept/testdata/markers.ts b/cmd/sin-code/internal/sindept/testdata/markers.ts new file mode 100644 index 00000000..4c60851f --- /dev/null +++ b/cmd/sin-code/internal/sindept/testdata/markers.ts @@ -0,0 +1,6 @@ +// SPDX-License-Identifier: MIT +// Purpose: golden-file fixtures for sindept_test.go (typescript flavour). +// Do not reflow. + +// sin-debt: in-memory cache, upgrade: switch to redis when instances > 1 +export const fixturesAtTopOfFile = (): void => {}; diff --git a/cmd/sin-code/main.go b/cmd/sin-code/main.go index 845d4804..aa92b089 100644 --- a/cmd/sin-code/main.go +++ b/cmd/sin-code/main.go @@ -95,6 +95,10 @@ func init() { NewCatalogCmd(), // v3.18.0: `sin-code catalog` — unified tool catalog (issue #163, supersedes `hub` + `assets`) NewCompileSpecCmd(), // v3.21.0: `sin-code compile-spec` — declarative .sin-code.yml → hooks/verify/perm (issue #164) NewGrillCmd(), // v3.18.0: `sin-code grill` — native adversarial design-review (issue #141 fusion) + NewRtkCmd(), // rtk (Rust Token Killer) bridge (issue #123) + NewCodeGraphCmd(), // CodeGraph multi-language analysis bridge (issue #126) + NewSpecCmd(), // Spec-Layer: *.spec.md contracts (issue #122) + NewDebtCmd(), // issue #177: sin-debt marker manager (`// sin-debt: , upgrade: `) internal.InstinctCmd, internal.HooksCmd, internal.AssetsCmd, internal.EvalCmd, internal.PRPCmd, // continuous learning + lifecycle hooks + asset harvest + eval + prp workflow ) diff --git a/docs/sin-debt-convention.md b/docs/sin-debt-convention.md new file mode 100644 index 00000000..ebbb17a8 --- /dev/null +++ b/docs/sin-debt-convention.md @@ -0,0 +1,166 @@ +# The sin-debt Marker Convention (issue #177) + +This document is the author-facing reference for the `sin-debt:` marker +convention adopted in SIN-Code. SIN-Code adopted ponytail's `ponytail:` +marker convention as the canonical way to name **intentional complexity** +in source code. The scanner (`internal/sindept`) and the CLI +(`sin-code debt`) operate on the format below. + +## Format + +```text +// sin-debt: , upgrade: +// sin-debt: # upgrade is OPTIONAL but RECOMMENDED +``` + +Where: + +- `` describes **what is limited**. Free-text but should match + one of the [default reasons](#default-reasons) when possible: + `global mutex`, `O(n²) scan`, `hand-rolled retry`, `in-memory cache`, + `synchronous I/O`, `polling`, etc. +- `` describes **when to revisit**. Free-text but should match + one of the [default upgrade triggers](#default-upgrade-triggers) when + possible: + `when throughput > N req/s`, `when n > 100`, + `when the upstream API stabilises`, etc. + +The two clauses are separated by a single comma. The upgrade clause is +optional — markers without one are flagged as `rot-risk` and trip a CI +gate at the configured threshold. + +## Comment families + +The parser recognises every common comment family: + +| Family | Languages | +|--------|-----------| +| `// ...` | Go, Rust, JavaScript, TypeScript, Kotlin, Swift, ... | +| `# ...` | Python, Shell, YAML, TOML, Ruby (line), ... | +| `-- ...` | SQL, Lua, Haskell | +| `/* ... */` | C, C++, CSS, Java, Go (rare), ... | +| `` | HTML, XML, Markdown | + +Block-comment closers (`*/`, `-->`) are stripped silently from the +captured reason/upgrade clauses so the same byte sequence is matched +regardless of comment style. + +## Examples + +### Go — acknowledged shortcut + +```go +// Recompute the entire cache on each request. Cheap for now; expensive +// above 1k users. Switch to incremental updates when the load profile +// shows it. +// sin-debt: global mutex, upgrade: per-account locks when throughput > 1k req/s +func recomputeCache() { ... } +``` + +### Python — known ceiling + +```python +# sin-debt: polling timer ticks, upgrade: switch to fsnotify when file count > 100 +def watch_config_dir(): + while not_stopped(): + refresh() + time.sleep(0.5) +``` + +### Rust — short-form (no upgrade; flagged rot-risk) + +```rust +// sin-debt: hand-rolled retry loop (TODO: revisit) +fn send_with_retry(...) { ... } +``` + +This is recognised but immediately flagged in `sin-code debt stats` +under `Rot-risk markers`. The CI threshold (`max_no_upgrade = 50` +default) controls when such markers fail the build. + +## Markers in prose comments + +The parser is line-anchored and multi-line aware. A line that *talks +about* the convention (e.g. inside a docs block or README) is NOT a +marker because it does not satisfy the comment-opener regex. Quotes in +backticks (`// sin-debt:`) inside still-running prose also do not match +because the surrounding `// ` line does not start with the marker +token. + +## Author checklist + +Before committing a shortcut, make sure the marker: + +1. Names the **ceiling** — what cannot grow. +2. Names the **trigger** — when to revisit. Avoid "soon" / "when needed" + unless "needed" is operationalised in the trigger. +3. Sits above (or below, ≤40 lines) the declaration it documents. +4. Lists a Symbol — the `nextSymbol` resolver will pick up the nearest + function/class automatically; you do not need to duplicate it. + +## CI integration + +```yaml +# .github/workflows/ci.yml — humans don't need to know; CI does. +- run: sin-code debt check --require-upgrade +``` + +Or with a soft threshold: + +```toml +# .sin-code/debt-policy.toml +[sin-debt] +max_no_upgrade = 50 # soft ceiling — fail above this +require_upgrade = false +``` + +## See also + +- `cmd/sin-code/debt_cmd.doc.md` — CLI reference. +- `cmd/sin-code/internal/sindept/sindept.doc.md` — package internals. +- Issue #179 — downstream complexity auditor ("approved shortcut" check). +- Issue #180 — audit-engine (`sin-code review --complexity`). + +## Default reasons + +| Reason | When to use | +|--------|-------------| +| `global mutex` | Single mutex protects multi-key state; per-key locks are the upgrade. | +| `O(n²) scan` | Linear scan over <100 elements; map lookup is the upgrade. | +| `hand-rolled retry` | Loop + sleep; cenkalti/backoff is the upgrade. | +| `hand-rolled backoff` | In-house retry scaling; cenkalti/backoff is the upgrade. | +| `in-memory cache` | In-process map; redis is the upgrade. | +| `in-process queue` | Go channel; redis stream / NATS is the upgrade. | +| `tied loop` | Two loops coupled across files; one function with rich return is the upgrade. | +| `manual json encode` | fmt.Sprintf; json.Marshal is the upgrade. | +| `manual json decode` | strings.Split; json.Unmarshal is the upgrade. | +| `disabled keepalive` | TCP layer off; connection cost is the upgrade. | +| `single-threaded worker` | One goroutine drains the queue; worker pool is the upgrade. | +| `polling` | sleep + check; fsnotify / inotify is the upgrade. | +| `polling-only watcher` | Same as `polling`. | +| `synchronous I/O` | Inline web calls; async pipeline is the upgrade. | +| `synchronous fan-out` | Sequential calls; goroutines + errgroup is the upgrade. | +| `blocking sleep` | time.Sleep; ticker or wake callback is the upgrade. | +| `rebuild-on-every-call` | Object rebuilt per call; pool / sync.Once is the upgrade. | +| `reload-on-import` | Re-import side effects; lazy construction is the upgrade. | +| `polling timer ticks` | Same as `polling`. | +| `best-effort retry` | When retry success is best-effort (cheap, idempotent). | +| `best-effort dedup` | When de-duplication is best-effort (cheap, idempotent). | + +## Default upgrade triggers + +| Slug | Phrase | +|------|--------| +| `throughput` | when throughput exceeds threshold | +| `scale` | when N exceeds threshold | +| `latency` | when latency exceeds threshold | +| `errors` | when error rate is non-trivial | +| `main` | when the upstream API stabilises | +| `stable` | when the upstream API stabilises | +| `context` | when context cancellation is required | +| `rswitch` | switch to when threshold breached | + +The `dswitch` / `rswitch` family invites the healthy habit of writing +the upgrade as a concrete switch instruction, not a vague "improve +later". A specific upgrade (`switch to redis when instances > 1`) gives +the next developer a clear target.