feat: M10 LENS — composable view projections#250
Merged
flyingrobots merged 7 commits intomainfrom Feb 17, 2026
Merged
Conversation
Implement M10 LENS milestone — composable view projections that post-process ViewResults via left-to-right chaining. New modules: - src/dag.js: Pure DAG utilities (adjacency, topoSort, cycles, walkChain, findRoots) - src/lens.js: Lens registry + composition engine (defineLens, composeLenses) - src/lenses/core.js: 5 built-in lenses (incomplete, frontier, critical-path, blocked, parallel) Changes: - renderView() accepts options.lenses for lens post-processing - needsProperties propagates through lens chain - CLI supports colon syntax: git mind view roadmap:incomplete:frontier - JSON output includes lenses array when lenses are applied - views.js blockers/onboarding refactored to use shared DAG utils - Lens API exported from src/index.js - JSON Schema contract: docs/contracts/cli/view-lens.schema.json Tests: 458 passing (46 new) across 24 files
Contributor
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds a composable lens system: pure DAG utilities, a lens registry and composer, five built-in lenses, CLI colon-delimited view:lens syntax and JSON contract/schema, view integration applying lenses post-filtering, comprehensive tests, and CI/docs/governance tooling. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant CLI as CLI (bin/git-mind.js)
participant Cmd as Commands (src/cli/commands.js)
participant Views as Views (src/views.js)
participant Lens as Lens (src/lens.js)
participant Graph as Graph/Renderer
User->>CLI: git mind view roadmap:incomplete:frontier --json
CLI->>Cmd: forward viewSpec "roadmap:incomplete:frontier"
Cmd->>Cmd: split → base="roadmap", lenses=["incomplete","frontier"]
Cmd->>Views: renderView(graph, "roadmap", { lenses: [...] , json:true })
Views->>Views: run base view.filterFn → viewResult
Views->>Lens: composeLenses(["incomplete","frontier"])
Lens->>Lens: build composedFn (left-to-right)
Lens-->>Views: composedFn
Views->>Graph: composedFn(viewResult, nodeProps?)
Graph->>Graph: apply "incomplete" then "frontier"
Graph-->>Views: transformed viewResult (nodes, edges, lenses, meta)
Views-->>Cmd: final formatted result
Cmd-->>User: output JSON including `lenses`
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related issues
Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 8
🤖 Fix all issues with AI agents
In `@src/cli/commands.js`:
- Around line 95-98: Split of viewSpec into segments can produce trailing empty
lens names (e.g., 'roadmap:' -> ['',]) which then flow into lensNames and cause
composeLenses to error; update the parsing so after creating segments from
viewSpec you set viewName = segments[0] and set lensNames =
segments.slice(1).filter(s => s !== '' && s != null) (or equivalent trimming) so
empty strings are removed before calling composeLenses; ensure this change is
applied where viewSpec, segments, viewName, lensNames and composeLenses are
used.
- Around line 113-118: The code currently builds payload as { viewName,
...result } even though renderView returns a ViewResult (nodes, edges, optional
meta) and cannot include viewName; to be defensively safe reverse the spread so
result properties cannot be accidentally overwritten by viewName — change the
payload creation in the json branch to { ...result, viewName } (keep the
conditional lensNames assignment and call to outputJson('view', payload)
unchanged); reference the functions/values renderView, ViewResult, viewName,
lensNames, and outputJson when locating the change.
In `@src/lens.js`:
- Around line 101-119: The current captureBuiltIns only records names in
builtInNames so resetLenses can't restore overwritten definitions; change
captureBuiltIns to save a map of full LensDefinition entries (e.g.,
builtInDefinitions = new Map(registry) or similar) and update resetLenses to
restore registry entries from builtInDefinitions for any name present there (and
remove any non-built-in entries), referencing the registry, captureBuiltIns,
resetLenses, and the LensDefinition objects to ensure overwritten built-ins are
reverted to their original definitions during test cleanup.
- Around line 31-43: defineLens currently overwrites existing entries (including
built-ins) in registry; add a guard that detects when registry.has(name) and the
existing entry is from the built-in set (e.g., maintain a BUILTIN_LENSES Set of
names or mark built-in entries with a flag like isBuiltin) and then either throw
or at minimum console.warn before overwriting; update defineLens to check that
marker (or BUILTIN_LENSES.has(name)) and refuse/warn, and ensure resetLenses
still restores originals by relying on the BUILTIN_LENSES definitions rather
than just name checks.
In `@src/lenses/core.js`:
- Around line 45-78: The critical-path lens currently uses walkChain (which
DFS-collects all reachable nodes) and then chooses the root with the most
descendants, producing side-branches instead of the true longest path; replace
this with a longest-path-in-DAG algorithm: use buildAdjacency/forward and
findRoots to get the DAG, compute a topological order, then run dynamic
programming to compute the longest path length and predecessor for each node and
reconstruct the single longest path for each root (pick the global max), return
that path's nodes and edges; also fix or remove the stale "fall back to topo
sort of all nodes" comment and ensure the fallback returns the topo-sorted
nodes/edges if you keep that behavior.
- Line 7: The import list in src/lenses/core.js includes an unused symbol
topoSort; remove topoSort from the import statement that currently reads "import
{ buildAdjacency, topoSort, findRoots, walkChain } from '../dag.js';" so it
becomes only the used symbols (buildAdjacency, findRoots, walkChain); verify
there are no other references to topoSort in this module before committing.
In `@src/views.js`:
- Around line 486-490: Add a brief inline comment above the side-effect import
'./lenses/core.js' explaining that ES module imports are hoisted so core.js
executes before the rest of this module, and that captureLensBuiltIns() is
intentionally called after view registration to snapshot the registry; reference
the import './lenses/core.js' and the call captureLensBuiltIns() so future
maintainers understand the ordering intent.
In `@test/lens.test.js`:
- Around line 227-239: The test "incomplete:frontier chains two lenses" should
also assert that intermediate node 'task:b' is excluded after composing lenses;
update the test in the it block named 'incomplete:frontier chains two lenses'
(which uses createEdge, setNodeProperty and renderView) to add an assertion that
result.nodes does not contain 'task:b' so the composed behavior of the
'incomplete' and 'frontier' lenses is fully validated.
src/lenses/core.js
Outdated
| */ | ||
|
|
||
| import { defineLens } from '../lens.js'; | ||
| import { buildAdjacency, topoSort, findRoots, walkChain } from '../dag.js'; |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# Check if topoSort is used anywhere in src/lenses/core.js
rg -n 'topoSort' src/lenses/core.jsRepository: neuroglyph/git-mind
Length of output: 140
🏁 Script executed:
# Get the import statement and surrounding lines for context
sed -n '1,15p' src/lenses/core.jsRepository: neuroglyph/git-mind
Length of output: 613
Remove unused topoSort import.
The topoSort function is imported from ../dag.js on line 7 but is never used in any of the five lens definitions in this file.
Suggested fix
-import { buildAdjacency, topoSort, findRoots, walkChain } from '../dag.js';
+import { buildAdjacency, findRoots, walkChain } from '../dag.js';📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| import { buildAdjacency, topoSort, findRoots, walkChain } from '../dag.js'; | |
| import { buildAdjacency, findRoots, walkChain } from '../dag.js'; |
🤖 Prompt for AI Agents
In `@src/lenses/core.js` at line 7, The import list in src/lenses/core.js includes
an unused symbol topoSort; remove topoSort from the import statement that
currently reads "import { buildAdjacency, topoSort, findRoots, walkChain } from
'../dag.js';" so it becomes only the used symbols (buildAdjacency, findRoots,
walkChain); verify there are no other references to topoSort in this module
before committing.
Add project governance and architectural foundation documents: - ARCHITECTURE.md: 12 engineering laws - REVIEW_RUBRIC.md: MUST/SHOULD review criteria - RISK_REGISTER.md + RISK_REVIEW_CHECKLIST.md: risk management - VISION_NORTH_STAR.md: product thesis and identity - ADR-0001: worktree independence + materialization - ADR-0002: graph-native content + sync workspace bridge - PR template with MUST checklist - CI workflows: architecture-gates.yml, check-pr-template.yml - Pre-commit hook + setup script - .gitignore example for materialized artifacts - Move dogfood-session.md to docs/notes/
Contributor
There was a problem hiding this comment.
Actionable comments posted: 12
🤖 Fix all issues with AI agents
In @.githooks/pre-commit:
- Around line 1-3: Resolve the hook script to the repository root before
invoking the checklist script to avoid edge-case failures: modify the pre-commit
hook to determine the repo root (e.g., via git rev-parse --show-toplevel or
deriving dirname "$0") and then call the checklist using that absolute/anchored
path instead of plain "scripts/review-checklist.sh" so the invocation always
targets the repository's scripts directory.
In @.github/pull_request_template.md:
- Around line 126-133: Under the "Contract/Schema" section the example block is
missing a closing code fence so the snippet renders as plain text; update the
".github/pull_request_template.md" under the "Contract/Schema" header by
wrapping the example lines ("# e.g." and "npm run validate:schemas") in a fenced
code block (use ```bash before the example and a closing ``` after) so the
command is rendered as a proper code block.
- Around line 182-207: The Reviewer Quick Verdict Block contains duplicated
sections: the second "MUST (Hard Gates)" and "SHOULD (Quality)" groups are
repeated; remove the redundant duplicate so the template only defines each
verdict block once (keep the first "MUST (Hard Gates)" and first "SHOULD
(Quality)" groups and delete the repeated ones). Update the block under the
"Reviewer Quick Verdict Block" header to list each of the three verdict options
(PASS, CONDITIONAL, FAIL) once per MUST/SHOULD heading to eliminate confusion.
In @.github/workflows/architecture-gates.yml:
- Around line 31-35: The workflow is writing BASE_REF_SHA using
github.event.before which can be the all-zero null SHA on first-time branch
pushes; update the logic that writes base_sha/head_sha so that if
github.event.before == "0000000000000000000000000000000000000000" you set
base_sha to "${{ github.sha }}^" (i.e., the parent of the current commit)
instead of the null SHA, otherwise keep github.event.before; alternatively,
handle the null- SHA check inside the script's changed_files() function, but
ensure the workflow sets BASE_REF_SHA to a valid ref (use github.sha^ when
before is null) so git diff calls do not fail.
In @.github/workflows/check-pr-template.yml:
- Around line 115-129: The validation builds a RegExp from field labels in
requiredExFields (used in the missingEx filter with new RegExp(`${f}\\s*$`,
"m")), but labels like "ADR clause(s) deviated from:" contain regex
metacharacters and must be escaped; update the missingEx check to escape
regex-special characters in each label before constructing the RegExp (e.g.,
add/use an escapeRegex utility or otherwise replace special chars) so new RegExp
receives a safe literal string, keeping the existing logic that checks for
labels present and not followed only by whitespace in exText.
- Around line 85-98: The regex used to detect checked PR template boxes is
corrupted; update the checkedPattern construction in the block that iterates
requiredChecked (where variables checkedPattern, requiredChecked, unchecked, and
fail are used) to match the literal "- [x] <label>" pattern by using an escaped
bracket regex like `- \[x\] ` concatenated with the existing label escaping (the
label.replace(/[.*+?^${}()|[\]\\]/g, '\\$&') part) and keep the "i" flag for
case-insensitivity so the checkbox detection works correctly.
In `@docs/adr/ADR-0001.md`:
- Around line 218-257: Convert the plain-text closing section titles into
Markdown headings consistent with the rest of the document: change "Invariants
Adopted", "Consequences", "Positive", "Costs", and "Follow-up Work Items" to use
the same heading syntax (e.g., prepend "## " or the appropriate existing heading
level) so they render as headings in Obsidian and match the document's
structure; update any nested subsection titles ("Positive" and "Costs") to the
correct subordinate heading level (e.g., "### ") to preserve hierarchy.
In `@scripts/review-checklist.sh`:
- Around line 1-2: The script runs Check 1 and Check 3 over all tracked files
via git ls-files which can be slow; change the checks to operate on only
staged/changed files when invoked as a pre-commit hook by deriving the file list
from git diff --cached --name-only (fall back to git ls-files for CI or when no
staged files are present). Update the logic in scripts/review-checklist.sh
around the Check 1 (forbidden path scan) and Check 3 (generated marker +
file|grep scan) blocks to accept a file list variable (e.g., FILES_TO_CHECK) and
use git diff --cached --name-only | tr '\n' ' ' to populate it for pre-commit,
while keeping the existing git ls-files behavior as a fallback for full-repo
runs.
- Around line 36-57: The comment and defaults disagree about separators: update
the config docs and any matching helper to be consistent by choosing one format
(recommended: keep the current defaults as a single extended regex using '|'
alternation) and change the comment "colon-separated regex list" to something
like "single extended regex; use '|' for alternation"; apply this change to the
comments for FORBIDDEN_TRACKED_PATHS_REGEX, PUBLISH_ALLOWLIST_REGEX,
GENERATED_MARKER_REGEX, CONTRACT_DIR_REGEX and CORE_PATH_REGEX and also update
any helper that parses these vars (e.g., matches_any_regex_rule) to treat the
value as a single ERE rather than a colon-separated list.
- Around line 75-86: Rename the parameter "rules" in the function
matches_any_regex_rule to avoid shadowing the array named "rules" used in
in_allowlist; for example change the parameter to "rules_str" (or
"rules_param"), update the local variable references (IFS read into arr from
"$rules_str"), and update any callers that pass the old argument name so they
pass the same value — ensure only the parameter name is changed, keep function
behavior identical, and run shellcheck to confirm SC2178/SC2128 are resolved.
- Around line 165-188: The CONTRACT_DIR_REGEX used by the contract/schema
hygiene check (involving variables CONTRACT_DIR_REGEX, contract_changed,
schema_changed and the changed_files loop) is too broad and matches any file
under src/cli/ or src/api/, causing false failures; update the check by either
(A) narrowing CONTRACT_DIR_REGEX to target only true contract surface files
(e.g., specific filename patterns for formatters/serializers or directories that
export contract types) so only those changes increment contract_changed, or (B)
change the failure branch to emit a non-blocking warning instead of calling fail
when contract_changed>0 and schema_changed==0; implement one of these in the
while-loop/conditional that evaluates contract_changed and schema_changed so
non-contract edits no longer trigger hard failures.
- Around line 88-103: The function changed_files() is being executed multiple
times causing redundant git calls; change it to compute and cache its result on
first call and return the cached value on subsequent calls (e.g., memoize into a
global/local variable inside changed_files), so callers (checks 2, 4, 5, 6) read
the cached list instead of re-running git diff/merge-base; update changed_files
to set a variable like CHANGED_FILES (or a local cached_files) when empty,
populate it with the appropriate git diff logic currently in changed_files, and
have the function echo/return that variable on every call.
Comment on lines
+182
to
+207
| ## Reviewer Quick Verdict Block (for maintainers) | ||
|
|
||
| ### MUST (Hard Gates) | ||
|
|
||
| • PASS | ||
| • CONDITIONAL | ||
| • FAIL | ||
|
|
||
| ### SHOULD (Quality) | ||
|
|
||
| • PASS | ||
| • CONDITIONAL | ||
| • FAIL | ||
|
|
||
| ### MUST (Hard Gates) | ||
|
|
||
| • PASS | ||
| • CONDITIONAL | ||
| • FAIL | ||
|
|
||
| ### SHOULD (Quality) | ||
|
|
||
| • PASS | ||
| • CONDITIONAL | ||
| • FAIL | ||
|
|
Contributor
There was a problem hiding this comment.
Remove the duplicated MUST/SHOULD verdict blocks.
The Reviewer Quick Verdict Block repeats MUST/SHOULD twice, which is redundant and confusing for maintainers.
🧹 Proposed fix
-### MUST (Hard Gates)
-
- • PASS
- • CONDITIONAL
- • FAIL
-
-### SHOULD (Quality)
-
- • PASS
- • CONDITIONAL
- • FAIL
-
-### MUST (Hard Gates)
+### MUST (Hard Gates)
• PASS
• CONDITIONAL
• FAIL
### SHOULD (Quality)
• PASS
• CONDITIONAL
• FAIL📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| ## Reviewer Quick Verdict Block (for maintainers) | |
| ### MUST (Hard Gates) | |
| • PASS | |
| • CONDITIONAL | |
| • FAIL | |
| ### SHOULD (Quality) | |
| • PASS | |
| • CONDITIONAL | |
| • FAIL | |
| ### MUST (Hard Gates) | |
| • PASS | |
| • CONDITIONAL | |
| • FAIL | |
| ### SHOULD (Quality) | |
| • PASS | |
| • CONDITIONAL | |
| • FAIL | |
| ## Reviewer Quick Verdict Block (for maintainers) | |
| ### MUST (Hard Gates) | |
| • PASS | |
| • CONDITIONAL | |
| • FAIL | |
| ### SHOULD (Quality) | |
| • PASS | |
| • CONDITIONAL | |
| • FAIL |
🤖 Prompt for AI Agents
In @.github/pull_request_template.md around lines 182 - 207, The Reviewer Quick
Verdict Block contains duplicated sections: the second "MUST (Hard Gates)" and
"SHOULD (Quality)" groups are repeated; remove the redundant duplicate so the
template only defines each verdict block once (keep the first "MUST (Hard
Gates)" and first "SHOULD (Quality)" groups and delete the repeated ones).
Update the block under the "Reviewer Quick Verdict Block" header to list each of
the three verdict options (PASS, CONDITIONAL, FAIL) once per MUST/SHOULD heading
to eliminate confusion.
Comment on lines
+36
to
+57
| # Config (override via env in CI or local shell) | ||
| # ----------------------------------------------------------------------------- | ||
|
|
||
| # Paths that MUST NEVER be tracked as normal source artifacts. | ||
| # Colon-separated regex list. | ||
| FORBIDDEN_TRACKED_PATHS_REGEX="${FORBIDDEN_TRACKED_PATHS_REGEX:-^\\.gitmind/materialized/|^\\.gitmind/cache/|^dist/materialized/|^tmp/materialized/}" | ||
|
|
||
| # Optional allowlist for publishable generated artifacts. | ||
| # If non-empty, generated files outside allowlist fail. | ||
| # Colon-separated regex list. | ||
| PUBLISH_ALLOWLIST_REGEX="${PUBLISH_ALLOWLIST_REGEX:-^docs/published/}" | ||
|
|
||
| # Marker used in generated files; if detected in tracked files outside allowlist => fail | ||
| GENERATED_MARKER_REGEX="${GENERATED_MARKER_REGEX:-^x-gitmind-generated:|^# Generated by Git Mind|^<!-- Generated by Git Mind -->}" | ||
|
|
||
| # Contract directories | ||
| SCHEMA_DIR="${SCHEMA_DIR:-contracts/schemas}" | ||
| CONTRACT_DIR_REGEX="${CONTRACT_DIR_REGEX:-^src/(cli|api|contracts)/|^contracts/}" | ||
|
|
||
| # Basic patterns to detect hidden worktree coupling in core paths | ||
| WORKTREE_COUPLING_PATTERN="${WORKTREE_COUPLING_PATTERN:-process\\.cwd\\(|fs\\.readFileSync\\(|fs\\.readFile\\(|git\\s+rev-parse\\s+--show-toplevel|git\\s+status}" | ||
| CORE_PATH_REGEX="${CORE_PATH_REGEX:-^src/(domain|core|engine|materialization)/}" |
Contributor
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Config comments say "colon-separated regex list", but the defaults use | (regex alternation) within a single pattern.
Both work—| alternation inside [[ =~ ]] and grep -E is valid, and the : split in matches_any_regex_rule just yields one element—but someone overriding these variables may be confused about the intended format. Clarify in the comments which separator to use, or pick one approach consistently.
🤖 Prompt for AI Agents
In `@scripts/review-checklist.sh` around lines 36 - 57, The comment and defaults
disagree about separators: update the config docs and any matching helper to be
consistent by choosing one format (recommended: keep the current defaults as a
single extended regex using '|' alternation) and change the comment
"colon-separated regex list" to something like "single extended regex; use '|'
for alternation"; apply this change to the comments for
FORBIDDEN_TRACKED_PATHS_REGEX, PUBLISH_ALLOWLIST_REGEX, GENERATED_MARKER_REGEX,
CONTRACT_DIR_REGEX and CORE_PATH_REGEX and also update any helper that parses
these vars (e.g., matches_any_regex_rule) to treat the value as a single ERE
rather than a colon-separated list.
- Filter empty strings from colon-split (handles trailing colon edge case) - Fix critical-path lens: use DP longest-path algorithm instead of DFS walkChain - resetLenses() now restores overwritten built-in definitions (not just names) - Remove unused walkChain import from lenses/core.js - Add explanatory comment for side-effect import ordering in views.js - Add task:b exclusion assertion to composition test
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In `@src/lenses/core.js`:
- Line 7: The import list in src/lenses/core.js includes an unused symbol
findRoots; remove findRoots from the import statement (leaving buildAdjacency
and topoSort) so the module no longer imports an unused identifier and avoids
lint warnings.
In `@test/lens.test.js`:
- Around line 53-59: Add a test that overwrites a built-in lens, verifies the
overwrite changed behavior, calls resetLenses(), and then verifies the original
implementation is restored; specifically, use defineLens('incomplete',
differentFn) to replace the built-in, assert the behavior or return value from
the overwritten 'incomplete' differs from the known built-in behavior, call
resetLenses(), and then assert 'incomplete' returns the original/built-in result
again to exercise the builtInDefs snapshot logic in src/lens.js (use the
existing helpers listLenses/defineLens/resetLenses to locate spots).
- cli/commands.js: reverse spread order to { ...result, viewName } so
renderView properties cannot be silently overwritten (#250)
- lens.js: warn when defineLens overwrites a captured built-in (#250)
- lenses/core.js: remove unused findRoots import (#250)
- test/lens.test.js: add test verifying resetLenses restores
overwritten built-in definition (#250)
- .githooks/pre-commit: resolve repo root via git rev-parse before
invoking review-checklist.sh to avoid relative-path failures (#250)
- pull_request_template.md: add missing code fence in Contract/Schema section; remove duplicated MUST/SHOULD verdict blocks (#250) - architecture-gates.yml: handle all-zero null SHA on first-time branch push by falling back to github.sha^ (#250) - check-pr-template.yml: fix corrupted checkbox regex (bracket escape mangled); escape regex metacharacters in missingEx field labels (#250) - ADR-0001.md: convert plain-text section titles (Invariants Adopted, Consequences, Positive, Costs, Follow-up Work Items) to Markdown headings for consistent rendering (#250) - review-checklist.sh: memoize changed_files() to avoid redundant git calls; use staged files when running as pre-commit hook; rename 'rules' param to 'rules_str' to avoid outer variable shadowing; narrow CONTRACT_DIR_REGEX to avoid false positives on src/cli/ (#250)
Owner
Author
|
@coderabbitai review |
Contributor
✅ Actions performedReview triggered.
|
Contributor
There was a problem hiding this comment.
Actionable comments posted: 12
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.github/pull_request_template.md:
- Around line 100-103: Update the example test command in the pull request
template so it matches the project's JS convention: replace the TypeScript glob
"test/materialization/*.test.ts" with the JavaScript glob
"test/materialization/*.test.js" in the code block (the line showing `npm test
-- test/materialization/*.test.ts`) so the template aligns with the project's
`**/*.{js,mjs}` guideline.
- Around line 95-144: The Test Plan subsections ("Integration", "Determinism",
"Contract/Schema", "Policy Gates", plus "Security / Trust Impact", "Performance
Impact", "Observability / Debuggability", "Operational Notes") use tab-indented
bullets (•) which break checkbox functionality; update each subsection to use
GitHub checkbox syntax "- [ ]" for all list items (keeping the existing command
code blocks intact) so that "Integration", "Determinism", "Contract/Schema",
"Policy Gates" and the other listed headings present actionable checkboxes;
locate the headings by their exact titles in the PR template to change each "•"
item to a "- [ ]" item.
In @.github/workflows/architecture-gates.yml:
- Around line 43-57: The CONTRACT_DIR_REGEX environment variable in the workflow
is too broad and re-introduces false positives; update the CONTRACT_DIR_REGEX
value in the workflow to match the script's narrowed default used by
scripts/review-checklist.sh (i.e., remove the src/cli portion so it only matches
the intended contract paths such as src/api and contracts), ensuring
CONTRACT_DIR_REGEX aligns with the script's expected pattern to avoid flagging
src/cli/.
- Around line 31-40: The current fallback sets before="${{ github.sha }}^" which
can be invalid for the repo's initial commit; instead, check whether the parent
commit exists (e.g. verify "${{ github.sha }}^" via git rev-parse) and only
set/echo base_sha when that parent is valid—if the parent is missing, leave
base_sha unset/empty so the later changed_files/HEAD~1 HEAD fallback runs; keep
echo "head_sha=${{ github.sha }}" as-is and only modify the logic around
before/echo "base_sha=..." referring to the before variable and the base_sha
output.
In `@scripts/review-checklist.sh`:
- Around line 53-55: The CI override for CONTRACT_DIR_REGEX reintroduces src/cli
and omits docs/contracts; update the CI configuration that sets
CONTRACT_DIR_REGEX (the override in the architecture gates workflow) to match
the script's improved default:
^src/(contracts|api)/|^contracts/|^docs/contracts/ so src/cli/ remains excluded
and docs/contracts/ is included, preserving the narrowed contract-surface
detection used by the review-checklist.sh script.
In `@src/lenses/core.js`:
- Around line 82-99: The selection of endNode from dist is non-deterministic
when multiple nodes have the same maxDist; make it deterministic by applying an
explicit tie-break using the existing sorted array (or topoSort ordering): build
an index map from sorted nodes to their positions, then in the loop over dist
update endNode when d > maxDist or when d === maxDist and sortedIndex[n] <
sortedIndex[endNode]; update references to endNode, maxDist, dist, sorted and
pred accordingly and/or add a short comment explaining the chosen tie-break
strategy to document intent.
- Around line 115-127: The blocked lens (defineLens('blocked')) currently marks
a node as blocked if an edge with label 'blocks' has its to endpoint in nodeSet
but does not verify the from endpoint; update the loop over viewResult.edges to
require both nodeSet.has(e.from) and nodeSet.has(e.to) before adding to
blockedNodes (mirroring the parallel lens behavior), so only blocks asserted by
nodes present in the current view mark targets as blocked; ensure nodeSet,
blockedNodes and the edges filtering (resultSet usage) continue to operate
consistently after this change.
- Around line 56-66: The DP silently drops nodes involved in cycles; after
calling buildAdjacency(depEdges) and topoSort([...depNodes], forward) inspect
the topoSort result for remaining (cycle) nodes and, if remaining.length > 0,
add a concise user-facing warning into the returned meta (e.g. meta.warning or
meta.warnings) describing that nodes in remaining were excluded due to cycles
and include their identifiers; update any code paths that construct the return
value to include this meta entry so users understand why some depNodes may be
missing (refer to depEdges, depNodes, forward, topoSort, remaining, and sorted).
- Around line 46-54: The critical issue is that defineLens('critical-path') and
the 'parallel' lens use mixed viewResult.edges (labels 'depends-on' and
'blocks') directly with buildAdjacency, but 'depends-on' semantics are reversed;
fix both lenses by separating edges into dependsOnEdges and blocksEdges, map
dependsOnEdges to reversed direction (swap from/to) before combining with
blocksEdges, then pass the combined, directionally-consistent edge list to
buildAdjacency; update any local variables like depEdges and the analogous
selection in the 'parallel' lens to use the transformed edges so the DP
longest-path logic in buildAdjacency operates on consistent execution-order
edges.
- Around line 14-23: The lens implementations (e.g., defineLens('incomplete',
...), and the similar 'frontier', 'blocked', and 'parallel' lenses) currently
filter edges with OR logic (keep edge if nodeSet.has(e.from) ||
nodeSet.has(e.to)); change the edge filtering to AND logic so only edges with
both endpoints present in the filtered node set are kept (use
nodeSet.has(e.from) && nodeSet.has(e.to)). Update the same filter expression in
each lens's viewResult.edges handling to ensure result.edges forms a
self-contained subgraph consistent with critical-path behavior.
In `@test/lens.test.js`:
- Around line 75-126: The test suite is missing coverage for the branch where
composeLenses receives null/undefined; add a small test (in the composeLenses
describe block) that calls composeLenses(null) and composeLenses(undefined) and
asserts the returned composedFn is the identity (composedFn(input) === input or
deep-equals) and needsProperties is false, so the guard in composeLenses (the
"!lensNames || lensNames.length === 0" path) is exercised and locked down.
- Around line 174-193: Add a new test in the "critical-path lens" suite in
test/lens.test.js that constructs a branching DAG (e.g., createEdge(graph, {
source: 'task:a', target: 'task:b', type: 'depends-on' }); createEdge(graph, {
source: 'task:b', target: 'task:c', type: 'depends-on' }); createEdge(graph, {
source: 'task:a', target: 'task:d', type: 'depends-on' })) then call
renderView(graph, 'backlog', { lenses: ['critical-path'] }) and assert the
returned result.nodes includes the nodes of the longest single path (a→b→c) but
does not include the side branch node ('task:d'); this ensures the critical-path
DP selects the longest path rather than all descendants.
---
Duplicate comments:
In `@scripts/review-checklist.sh`:
- Around line 36-59: The comments for FORBIDDEN_TRACKED_PATHS_REGEX and
PUBLISH_ALLOWLIST_REGEX are misleading: update their inline comments to state
that the variable accepts a colon-separated list of EREs (each element may
itself use alternation with |) and mention that the helper
matches_any_regex_rule splits on ':' when evaluating these variables; also
adjust the GENERATED_MARKER_REGEX and CONTRACT_DIR_REGEX comments if needed to
match the same format and add a short example (e.g., "pattern1:pattern2" or a
single alternation "a|b") so someone overriding FORBIDDEN_TRACKED_PATHS_REGEX,
PUBLISH_ALLOWLIST_REGEX, or similar vars understands both forms are supported.
- Around line 162-175: The loop currently iterates all tracked files
(tracked_files) running file+grep per file which is slow; change it to operate
on a limited set when appropriate by using the existing changed_files
optimization: call changed_files first and fall back to tracked_files if that
returns empty (or add an env flag to force staged-only), then iterate that list
instead. Update the process substitution used by the while loop so it uses
changed_files (or the changed_files || tracked_files result) while keeping the
same checks around GENERATED_MARKER_REGEX, in_allowlist, marker_hits and the
fail call.
Comment on lines
+43
to
+57
| env: | ||
| BASE_REF_SHA: ${{ steps.shas.outputs.base_sha }} | ||
| HEAD_REF_SHA: ${{ steps.shas.outputs.head_sha }} | ||
|
|
||
| # Customize policy here (regex, colon-separated for multiple rules) | ||
| FORBIDDEN_TRACKED_PATHS_REGEX: "^\\.gitmind/materialized/|^\\.gitmind/cache/|^dist/materialized/|^tmp/materialized/" | ||
| PUBLISH_ALLOWLIST_REGEX: "^docs/published/" | ||
| GENERATED_MARKER_REGEX: "^x-gitmind-generated:|^# Generated by Git Mind|^<!-- Generated by Git Mind -->" | ||
|
|
||
| SCHEMA_DIR: "contracts/schemas" | ||
| CONTRACT_DIR_REGEX: "^src/(cli|api|contracts)/|^contracts/" | ||
| CORE_PATH_REGEX: "^src/(domain|core|engine|materialization)/" | ||
| WORKTREE_COUPLING_PATTERN: "process\\.cwd\\(|fs\\.readFileSync\\(|fs\\.readFile\\(|git\\s+rev-parse\\s+--show-toplevel|git\\s+status" | ||
| run: | | ||
| scripts/review-checklist.sh |
Contributor
There was a problem hiding this comment.
CONTRACT_DIR_REGEX in the workflow is broader than the script's default — re-introduces false positives.
This was flagged in the scripts/review-checklist.sh review. The workflow passes ^src/(cli|api|contracts)/|^contracts/ which includes src/cli/, but the script's own default was intentionally narrowed to exclude it.
🔧 Proposed fix — align with the script's narrowed default
- CONTRACT_DIR_REGEX: "^src/(cli|api|contracts)/|^contracts/"
+ CONTRACT_DIR_REGEX: "^src/(contracts|api)/|^contracts/|^docs/contracts/"📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| env: | |
| BASE_REF_SHA: ${{ steps.shas.outputs.base_sha }} | |
| HEAD_REF_SHA: ${{ steps.shas.outputs.head_sha }} | |
| # Customize policy here (regex, colon-separated for multiple rules) | |
| FORBIDDEN_TRACKED_PATHS_REGEX: "^\\.gitmind/materialized/|^\\.gitmind/cache/|^dist/materialized/|^tmp/materialized/" | |
| PUBLISH_ALLOWLIST_REGEX: "^docs/published/" | |
| GENERATED_MARKER_REGEX: "^x-gitmind-generated:|^# Generated by Git Mind|^<!-- Generated by Git Mind -->" | |
| SCHEMA_DIR: "contracts/schemas" | |
| CONTRACT_DIR_REGEX: "^src/(cli|api|contracts)/|^contracts/" | |
| CORE_PATH_REGEX: "^src/(domain|core|engine|materialization)/" | |
| WORKTREE_COUPLING_PATTERN: "process\\.cwd\\(|fs\\.readFileSync\\(|fs\\.readFile\\(|git\\s+rev-parse\\s+--show-toplevel|git\\s+status" | |
| run: | | |
| scripts/review-checklist.sh | |
| env: | |
| BASE_REF_SHA: ${{ steps.shas.outputs.base_sha }} | |
| HEAD_REF_SHA: ${{ steps.shas.outputs.head_sha }} | |
| # Customize policy here (regex, colon-separated for multiple rules) | |
| FORBIDDEN_TRACKED_PATHS_REGEX: "^\\.gitmind/materialized/|^\\.gitmind/cache/|^dist/materialized/|^tmp/materialized/" | |
| PUBLISH_ALLOWLIST_REGEX: "^docs/published/" | |
| GENERATED_MARKER_REGEX: "^x-gitmind-generated:|^# Generated by Git Mind|^<!-- Generated by Git Mind -->" | |
| SCHEMA_DIR: "contracts/schemas" | |
| CONTRACT_DIR_REGEX: "^src/(contracts|api)/|^contracts/|^docs/contracts/" | |
| CORE_PATH_REGEX: "^src/(domain|core|engine|materialization)/" | |
| WORKTREE_COUPLING_PATTERN: "process\\.cwd\\(|fs\\.readFileSync\\(|fs\\.readFile\\(|git\\s+rev-parse\\s+--show-toplevel|git\\s+status" | |
| run: | | |
| scripts/review-checklist.sh |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.github/workflows/architecture-gates.yml around lines 43 - 57, The
CONTRACT_DIR_REGEX environment variable in the workflow is too broad and
re-introduces false positives; update the CONTRACT_DIR_REGEX value in the
workflow to match the script's narrowed default used by
scripts/review-checklist.sh (i.e., remove the src/cli portion so it only matches
the intended contract paths such as src/api and contracts), ensuring
CONTRACT_DIR_REGEX aligns with the script's expected pattern to avoid flagging
src/cli/.
src/lenses/core.js: - Normalise depends-on edges to execution order (reverse direction) before building DAG adjacency in critical-path lens (#250) - Add deterministic tie-break for endNode selection using topo-sort index when multiple nodes share max distance (#250) - Expose cycle-exclusion warning in critical-path meta.warnings (#250) - Require both edge endpoints in nodeSet for blocked lens (was only checking the 'to' endpoint) (#250) - Switch edge filtering from OR to AND logic in incomplete/frontier/ blocked/parallel lenses for self-contained subgraph output (#250) test/lens.test.js: - Add critical-path branching DAG test (verifies side-branch excluded) - Add composeLenses(null/undefined) identity test (#250) .github/pull_request_template.md: - Fix example test glob (.test.ts → .test.js) (#250) - Convert all tab-indented bullet (•) items to GitHub checkbox syntax (- [ ]) for actionable PR review (#250) .github/workflows/architecture-gates.yml: - Sync CONTRACT_DIR_REGEX to match narrowed script default (#250) - Guard sha^ fallback: verify parent exists before using it; omit base_sha entirely for initial repo commits (#250) scripts/review-checklist.sh: - Check 3 now uses changed_files (falls back to tracked_files when empty) for faster pre-commit runs (#250) - Clarify config variable comments to describe ERE/colon-separated format unambiguously (#250)
Owner
Author
|
@coderabbitai review |
Contributor
✅ Actions performedReview triggered.
|
Contributor
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@scripts/review-checklist.sh`:
- Around line 160-188: The script silently falls back to scanning all tracked
files when changed_files returns empty; update the logic around _check3_files,
changed_files, and tracked_files to emit an informational log (use say or
similar) when the fallback is taken so users know a full-repo scan will occur,
e.g., detect the empty result from changed_files, call say "No changed files
detected; falling back to scanning all tracked files (this may be slow)”, then
proceed to set _check3_files="$(tracked_files)" and continue with the existing
loop that calls file and grep.
- Around line 93-122: The conditional currently calls git diff --cached
--name-only twice; to avoid the duplicate invocation capture its output once
into a local variable (e.g., cached_files) inside changed_files, test -n
"$cached_files" for the conditional, and set result="$cached_files" when true;
update references to the memoization symbols (_CHANGED_FILES_CACHE,
_CHANGED_FILES_COMPUTED) unchanged and only change the branch that currently
runs git diff --cached --name-only so you perform the command a single time.
- Line 178: The grep call using GENERATED_MARKER_REGEX passes the raw
colon-separated string verbatim which causes ':' to be treated as a literal;
update the check around the if grep -Eq "$GENERATED_MARKER_REGEX" "$f" to first
split GENERATED_MARKER_REGEX on ':' (same behavior as matches_any_regex_rule /
in_allowlist) and join into a single ERE suitable for grep (e.g., join with '|'
or build a grouped alternation) before calling grep -E; ensure any whitespace
trimming and empty-segment handling matches the existing splitting logic used by
matches_any_regex_rule and in_allowlist so colon-separated patterns work
correctly.
In `@src/lenses/core.js`:
- Around line 119-121: The current implementation overwrites meta.lens during
lens composition so only the last lens name remains; update the assignment logic
in src/lenses/core.js to preserve all applied lenses instead of last-writer-wins
by either (a) making meta.lens an array and pushing each lens name when a lens
runs, or (b) setting meta.lens only if it is undefined and relying on the
existing top-level lenses array for full history—pick the approach consistent
with the existing "lenses" JSON output and change the code that writes meta.lens
(refer to the meta.lens symbol and the lenses array used at view level) so
composed lenses are recorded rather than overwritten.
Comment on lines
+93
to
+122
| # Memoized changed_files: computed once and cached in _CHANGED_FILES_CACHE. | ||
| _CHANGED_FILES_CACHE="" | ||
| _CHANGED_FILES_COMPUTED=0 | ||
|
|
||
| changed_files() { | ||
| if [[ $_CHANGED_FILES_COMPUTED -eq 1 ]]; then | ||
| echo "$_CHANGED_FILES_CACHE" | ||
| return | ||
| fi | ||
| local result | ||
| # Prefer staged files when running as a pre-commit hook (no CI SHAs set). | ||
| if [[ -n "${BASE_REF_SHA:-}" && -n "${HEAD_REF_SHA:-}" ]]; then | ||
| result="$(git diff --name-only "$BASE_REF_SHA" "$HEAD_REF_SHA")" | ||
| elif [[ -n "$(git diff --cached --name-only 2>/dev/null)" ]]; then | ||
| # Pre-commit: only check staged files for speed | ||
| result="$(git diff --cached --name-only)" | ||
| else | ||
| # Local fallback against merge-base with main if exists | ||
| if git show-ref --verify --quiet refs/heads/main; then | ||
| local mb | ||
| mb="$(git merge-base HEAD main)" | ||
| result="$(git diff --name-only "$mb" HEAD)" | ||
| else | ||
| result="$(git diff --name-only HEAD~1 HEAD 2>/dev/null || true)" | ||
| fi | ||
| fi | ||
| _CHANGED_FILES_CACHE="$result" | ||
| _CHANGED_FILES_COMPUTED=1 | ||
| echo "$result" | ||
| } |
Contributor
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Memoized changed_files with staged-file preference looks solid.
Minor note: line 106 runs git diff --cached --name-only once for the conditional test and then again on line 108 for the result capture. This is a negligible cost for a git plumbing command, but if you want to tighten it:
♻️ Optional: avoid double invocation
- elif [[ -n "$(git diff --cached --name-only 2>/dev/null)" ]]; then
- # Pre-commit: only check staged files for speed
- result="$(git diff --cached --name-only)"
+ else
+ local cached
+ cached="$(git diff --cached --name-only 2>/dev/null || true)"
+ if [[ -n "$cached" ]]; then
+ result="$cached"🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@scripts/review-checklist.sh` around lines 93 - 122, The conditional currently
calls git diff --cached --name-only twice; to avoid the duplicate invocation
capture its output once into a local variable (e.g., cached_files) inside
changed_files, test -n "$cached_files" for the conditional, and set
result="$cached_files" when true; update references to the memoization symbols
(_CHANGED_FILES_CACHE, _CHANGED_FILES_COMPUTED) unchanged and only change the
branch that currently runs git diff --cached --name-only so you perform the
command a single time.
Comment on lines
+160
to
+188
| # ----------------------------------------------------------------------------- | ||
| # Check 3: Generated marker in changed files outside publish allowlist | ||
| # (uses changed_files for speed; falls back to tracked_files when empty) | ||
| # ----------------------------------------------------------------------------- | ||
| say "Scanning changed text files for generated marker outside allowlist..." | ||
| marker_hits=0 | ||
| _check3_files="$(changed_files)" | ||
| if [[ -z "$_check3_files" ]]; then | ||
| _check3_files="$(tracked_files)" | ||
| fi | ||
| while IFS= read -r f; do | ||
| [[ -z "$f" ]] && continue | ||
| [[ -f "$f" ]] || continue | ||
| # quick skip binary files | ||
| if file "$f" 2>/dev/null | grep -qiE 'binary|image|pdf|archive'; then | ||
| continue | ||
| fi | ||
|
|
||
| if grep -Eq "$GENERATED_MARKER_REGEX" "$f" 2>/dev/null; then | ||
| if ! in_allowlist "$f"; then | ||
| fail "Generated marker found outside publish allowlist: $f" | ||
| marker_hits=$((marker_hits+1)) | ||
| fi | ||
| fi | ||
| done <<< "$_check3_files" | ||
|
|
||
| if [[ $marker_hits -eq 0 ]]; then | ||
| pass "No generated-marker leakage outside publish allowlist." | ||
| fi |
Contributor
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Check 3 fallback to tracked_files scans the entire repo when no changed files are detected.
When running outside CI/pre-commit (e.g., manually with no staged changes and no merge-base), this silently falls back to scanning every tracked file — including running file and grep on each. This could be unexpectedly slow on larger repos and may surprise developers who run the script manually.
Consider at least logging when the fallback is used:
♻️ Suggested improvement
if [[ -z "$_check3_files" ]]; then
+ say "(no changed files detected — falling back to full tracked-file scan)"
_check3_files="$(tracked_files)"
fi🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@scripts/review-checklist.sh` around lines 160 - 188, The script silently
falls back to scanning all tracked files when changed_files returns empty;
update the logic around _check3_files, changed_files, and tracked_files to emit
an informational log (use say or similar) when the fallback is taken so users
know a full-repo scan will occur, e.g., detect the empty result from
changed_files, call say "No changed files detected; falling back to scanning all
tracked files (this may be slow)”, then proceed to set
_check3_files="$(tracked_files)" and continue with the existing loop that calls
file and grep.
| continue | ||
| fi | ||
|
|
||
| if grep -Eq "$GENERATED_MARKER_REGEX" "$f" 2>/dev/null; then |
Contributor
There was a problem hiding this comment.
grep -Eq receives the raw regex string without colon-splitting.
matches_any_regex_rule and in_allowlist split on : to support multiple patterns, but the grep -Eq call here passes $GENERATED_MARKER_REGEX verbatim. If a user configures it with colon-separated patterns (e.g. "^foo:^bar"), grep will treat : as a literal character instead of a pattern delimiter, silently failing to match.
The current default happens to work because it uses | alternation within a single ERE, but the documented interface promises colon-separation support.
🔧 Proposed fix — split and build a grep-compatible pattern
- if grep -Eq "$GENERATED_MARKER_REGEX" "$f" 2>/dev/null; then
+ # Convert colon-separated patterns to a single ERE with '|' alternation
+ _marker_re="${GENERATED_MARKER_REGEX//:/$'|'}"
+ if grep -Eq "$_marker_re" "$f" 2>/dev/null; then🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@scripts/review-checklist.sh` at line 178, The grep call using
GENERATED_MARKER_REGEX passes the raw colon-separated string verbatim which
causes ':' to be treated as a literal; update the check around the if grep -Eq
"$GENERATED_MARKER_REGEX" "$f" to first split GENERATED_MARKER_REGEX on ':'
(same behavior as matches_any_regex_rule / in_allowlist) and join into a single
ERE suitable for grep (e.g., join with '|' or build a grouped alternation)
before calling grep -E; ensure any whitespace trimming and empty-segment
handling matches the existing splitting logic used by matches_any_regex_rule and
in_allowlist so colon-separated patterns work correctly.
Comment on lines
+119
to
+121
| const chain = path.map((node, i) => ({ node, depth: i })); | ||
| const chainSet = new Set(path); | ||
| const edges = viewResult.edges.filter(e => chainSet.has(e.from) && chainSet.has(e.to)); |
Contributor
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
meta.lens gets overwritten during composition — consider whether this is intentional.
When lenses are composed (e.g., incomplete:frontier), each lens sets meta.lens to its own name, so only the last lens's name survives. If consumers need to know which lenses were applied, this loses information. The lenses array in the JSON output (added at the view level per the schema contract) likely covers this, but it's worth confirming that meta.lens being last-writer-wins is the intended behavior.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/lenses/core.js` around lines 119 - 121, The current implementation
overwrites meta.lens during lens composition so only the last lens name remains;
update the assignment logic in src/lenses/core.js to preserve all applied lenses
instead of last-writer-wins by either (a) making meta.lens an array and pushing
each lens name when a lens runs, or (b) setting meta.lens only if it is
undefined and relying on the existing top-level lenses array for full
history—pick the approach consistent with the existing "lenses" JSON output and
change the code that writes meta.lens (refer to the meta.lens symbol and the
lenses array used at view level) so composed lenses are recorded rather than
overwritten.
flyingrobots
added a commit
that referenced
this pull request
Feb 17, 2026
- Bump version 3.1.0 → 3.2.0 (M10 LENS is a minor feature addition) - Rewrite README: add lens/view section, full command coverage, import YAML format, commit directives, multi-repo, AI curation, JSON scripting, causal versioning framing; fix stale alpha version - CHANGELOG: add [3.2.0] entry documenting M10 LENS additions and all changes from PR #250 - Update GitHub repo description via gh CLI
32 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
src/dag.js(adjacency, topoSort, cycles, walkChain, findRoots)src/lens.js(mirrors views.js pattern)incomplete,frontier,critical-path,blocked,parallelgit mind view roadmap:incomplete:frontierProblem Statement
ADR Compliance (Required)
Relevant ADR(s)
Compliance Declaration
Exception Request (Required if deviating)
N/A
Architecture Laws Checklist (Hard Gates)
Canonical Truth & Context
--at,--observer,--trust) or deterministically defaulted.Determinism & Provenance
Artifact Hygiene
Contracts & Compatibility
Extension/Effects Safety (if applicable)
Scope Control
Governance docs are bundled because they were untracked local files that define the review/architecture standards this PR is evaluated against.
Backward Compatibility
lensesfield in view JSON output (additive only, not present when no lenses applied)git mind viewnow lists available lenses. Colon syntax is new, existing view commands unchanged.Test Plan (Required)
Unit
Integration
Determinism
Contract/Schema
Policy Gates
# CI runs architecture-gates.yml automaticallySecurity / Trust Impact
Performance Impact
Observability / Debuggability
--jsonoutput includeslensesarray when lenses are applied.git mind view(no args) now lists available lenses alongside views.Operational Notes
Linked Issues / Milestones
Reviewer Quick Verdict Block (for maintainers)
MUST (Hard Gates)
SHOULD (Quality)
Verdict
Summary by CodeRabbit
New Features
Bug Fixes
Tests
Documentation
Chores