Compile the Dakar workflow from typed modules (5.)#3
Conversation
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Sorry @leynos, your pull request is larger than the review limit of 150000 diff characters
|
@coderabbitai review |
✅ Action performedReview finished.
|
|
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:
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Plus Run ID: 📒 Files selected for processing (10)
Summary
ValidationFormatting, linting, strict type checking, documentation auditing, workflow freshness, deterministic repeated builds, ODW dry-run execution, 115 tests, and isolated live-review parity all pass. WalkthroughChangesWorkflow compilation and review runtime
Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 19 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (19 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 Biome (2.5.3)workflows/dakar-review.jsFile contains syntax errors that prevent linting: Line 954: Illegal return statement outside of a function Comment |
✅ Action performedReview finished.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
|
@coderabbitai Have the following now been resolved? tests/workflow-candidate-paths.test.mjs (1)
🧪 Proposed fixture fixconst TASK_GRAPH = [
{
taskId: 'source-1',
kind: 'source',
assignedModel: 'gpt-5.5/high',
files: [],
+ maxFindings: 30,
verificationPolicy: 'verify-all',
},
]🤖 Detailed instructionsVerify each finding against current code. Fix only still-valid issues, skip the In |
|
@coderabbitai Have the following failed checks now been resolved? If further work is required, please provide an AI agent prompt for the remaining work to be done to address these failures. Do not treat warnings as optional or aspirational. Where a change is out of scope for this PR, propose a GitHub issue unless one exists already. (Treat documentation and validation coverage as in scope). ❌ Failed checks (3 errors, 3 warnings)
|
|
@coderabbitai resume |
✅ Action performedReviews resumed. |
1 similar comment
✅ Action performedReviews resumed. |
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chain🏁 Script executed: #!/bin/bash
sed -n '1,80p' tests/workflow-candidate-paths.test.mjsLength of output: 2782 Confirmed. Line 20-50 area now defines |
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
Define the ODW-specific TypeScript compilation boundary, module ownership, freshness checks, and behaviour-preserving validation sequence. Record the architectural decision, update component and contributor documentation, and align the roadmap with the reviewed ExecPlan.
Record explicit implementation approval and move the living ExecPlan into progress with baseline gate evidence.
Pin the contributor TypeScript and esbuild toolchain, declare the ambient ODW surface, and test both configured-project and erasable-syntax constraints.
Add the fail-closed esbuild compiler, strict runtime manifest, atomic freshness checks, typed workflow entry, and trust-boundary regressions while preserving the canonical ODW artefact.
Read repository AGENTS.md content from the base Git object so reviewed working-tree changes cannot inject workflow instructions.
Split configuration, routing, task planning, candidates, and prompts into directly tested TypeScript modules while retaining orchestration in the generated workflow entry.
Synchronize the architecture, contributor practices, user runtime contract, roadmap, and living execution plan with the implemented TypeScript source split.
Close the execution plan and roadmap slice with deterministic-build, isolated live-recording, same-head skip, and final review evidence.
Validate trusted instructions and external workflow arguments, enforce bounded task and retry behaviour, and keep generated artefact checks portable across line endings. Add focused examples and properties for every corrected boundary, document record retry observability, and align contributor guidance with the compiled ODW source contract.
Align the two remaining section references with the existing British-English `Artefacts and notes` heading.
Add concise JSDoc contracts for the workflow entry, exported functions, constants, interfaces, and type aliases without changing generated runtime behaviour.
Resolve trusted instruction reads through an immutable commit and derive review-history destinations from trusted repository and state-root inputs. Apply deterministic source ordering before candidate caps, reject fractional chunk sizes, and omit misleading unscoped diff commands for empty tasks. Document the resulting contracts and add regression coverage for each review finding.
This comment was marked as resolved.
This comment was marked as resolved.
|
@coderabbitai resume |
✅ Action performedReviews resumed. |
Parse metadata banners structurally, detect assignment aliases, and reject conflicting model reasoning before workflow execution. Validate complete Record acknowledgements and SHA-1 or SHA-256 review heads, with focused regressions for each fail-closed boundary. Document the authored JSDoc coverage convention and updated validation evidence.
|
@coderabbitai review |
✅ Action performedReview finished.
|
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
Preserve Unicode candidate identity, reject mismatched or duplicate verifier responses, and reconcile accepted overflow before applying report limits. Keep synthesis metrics out of persisted workflow telemetry, retry incomplete Record acknowledgements, emit a leading generated module docstring, and align the completed ADR and ExecPlan wording.
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
Add a repository validation command with an explicit authored-source scope and enforce the 80 percent threshold through `make check`. Document the remaining internal workflow helpers and reconcile the completed ExecPlan with its approval history and final validation record.
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chain🏁 Script executed: #!/bin/bash
set -euo pipefail
echo '=== Target file outlines ==='
ast-grep outline src/workflows/dakar-review/candidates.ts --items all
ast-grep outline src/workflows/dakar-review/main.ts --items all
ast-grep outline src/workflows/dakar-review/schemas.ts --items all
ast-grep outline scripts/build-workflow.mjs --items all
ast-grep outline tests/workflow-candidate-paths.test.mjs --items all
ast-grep outline tests/workflow-orchestration.test.mjs --items all
echo '=== Candidate identity and verdict reduction ==='
rg -n -C 10 'candidateKey|normalizeCandidates|acceptedFromVerdicts|discardedFromVerdicts|candidateId' \
src/workflows/dakar-review/candidates.ts
echo '=== Verification, synthesis metrics, record predicate and retry ==='
rg -n -C 12 'scheduledCandidate|candidateId|synthesis|metrics|recordInput|isCompleteRecord|recordAttempts|sleep\(' \
src/workflows/dakar-review/main.ts
echo '=== Synthesis schema metrics ==='
rg -n -C 8 'SYNTHESIS_SCHEMA|metrics|additionalProperties' src/workflows/dakar-review/schemas.ts
echo '=== Generated artefact and meta source headers ==='
head -n 12 workflows/dakar-review.js
head -n 12 src/workflows/dakar-review/meta.js
echo '=== Targeted test evidence ==='
rg -n -i -C 5 'unicode|non.latin|chinese|cyrillic|candidateId|duplicate|mismatch|synthesis.*metric|metric.*synthesis|malformed|recordAttempts|three attempts|sleepDelays' \
tests/workflow-candidate-paths.test.mjs tests/workflow-orchestration.test.mjs tests/review-state.test.mjs
echo '=== Documentation headers and documented declarations ==='
for f in src/workflows/dakar-review/*.{ts,d.ts,js} bin/dakar-review.mjs scripts/build-workflow.mjs; do
test -f "$f" || continue
echo "--- $f ---"
sed -n '1,8p' "$f"
rg -n '^\s*/\*\*|^export (async )?function|^export (interface|type|const)|^(async )?function workflowMain' "$f" || true
done
echo '=== Documentation coverage tooling and ExecPlan wording ==='
rg -n -i -C 3 'docstring|documentation.coverage|coverage' package.json Makefile tests scripts docs AGENTS.md || true
rg -n -i -C 3 'status:|approval|approve|implementation|complete|115 tests' \
docs/execplans/compile-dakar-review-workflow.mdLength of output: 94885 🏁 Script executed: #!/bin/bash
set -euo pipefail
echo '=== Documentation audit implementation ==='
cat -n scripts/check-docstrings.mjs
echo '=== Audited source inventory ==='
fd -t f -e ts -e js . src/workflows/dakar-review bin | sort
echo '=== Functions, classes, exported types/interfaces/constants selected by source syntax ==='
for f in bin/dakar-review.mjs src/workflows/dakar-review/*.{ts,d.ts,js}; do
test -f "$f" || continue
echo "--- $f ---"
rg -n '^(export\s+)?(async\s+)?function\s+[A-Za-z_$][A-Za-z0-9_$]*|^export\s+(class|interface|type|const)\s+[A-Za-z_$][A-Za-z0-9_$]*' "$f" || true
done
echo '=== JSDoc immediately preceding declarations, including single-line forms ==='
for f in bin/dakar-review.mjs src/workflows/dakar-review/*.{ts,d.ts,js}; do
test -f "$f" || continue
echo "--- $f ---"
rg -n -U '/\*\*[\s\S]*?\*/\s*\n\s*(export\s+)?(async\s+)?function|/\*\*[\s\S]*?\*/\s*\n\s*export\s+(class|interface|type|const)' "$f" || true
done
echo '=== Approval-transition language in the completed ExecPlan ==='
sed -n '1,36p;145,185p;468,485p;995,1018p' docs/execplans/compile-dakar-review-workflow.mdLength of output: 49152
No AI-agent prompt is needed because no reported item remains open. |
|
@coderabbitai review |
✅ Action performedReview finished.
|
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
Reject audits that discover no symbols and cover the command boundary with empty, documented, and below-threshold fixtures. Recognize bind-wrapped injected primitive calls in the workflow source guard.
|
@coderabbitai resume |
✅ Action performedReviews resumed. |
|
@coderabbitai Have the following now been resolved? tests/workflow-build.test.mjs (1)
Proposed fix- if (ts.isPropertyAccessExpression(expression) && ['call', 'apply'].includes(expression.name.text)) {
+ if (ts.isPropertyAccessExpression(expression) && ['call', 'apply', 'bind'].includes(expression.name.text)) {As per coding guidelines, add a regression test for each bug fix. 🤖 Detailed instructionsVerify each finding against current code. Fix only still-valid issues, skip the In Source: Coding guidelines |
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chain🏁 Script executed: #!/bin/bash
set -euo pipefail
ast-grep outline tests/workflow-build.test.mjs --items all
echo "=== primitive-call analysis near the reported range ==="
sed -n '260,325p' tests/workflow-build.test.mjs
echo "=== primitive-boundary regression fixture and assertion ==="
rg -n -C 18 'unsafe|primitive|\.call|\.apply|\.bind|main\.ts' tests/workflow-build.test.mjsLength of output: 13976
The unsafe fixture at Line 313 includes: phase.bind(null)("Bound")The expected detected-call list at Line 317 includes the corresponding |
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
Return phase-specific failures when configuration, preparation, or synthesis agent calls reject instead of allowing those errors to escape the workflow. Exclude ambient declarations from runtime docstring coverage and pin the default audit scope with a regression fixture.
|
@coderabbitai review |
✅ Action performedReview finished.
|
|
@coderabbitai Have the following now been resolved? src/workflows/dakar-review/main.ts (1)
🤖 Detailed instructionsVerify each finding against current code. Fix only still-valid issues, skip the In Sources: Coding guidelines, Learnings |
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chain🏁 Script executed: #!/bin/bash
set -euo pipefail
echo '=== main workflow outline ==='
ast-grep outline src/workflows/dakar-review/main.ts --items all
echo '=== direct phase calls and error handling in TypeScript source ==='
rg -n -C 14 'config-resolve|state-prepare|synthesis|stage: .(config|prepare|synthesize).|agent\(' \
src/workflows/dakar-review/main.ts
echo '=== generated workflow counterparts ==='
rg -n -C 12 'config-resolve|state-prepare|synthesis|stage: .(config|prepare|synthesize).|agent\(' \
workflows/dakar-review.js
echo '=== orchestration harness and direct-phase failure tests ==='
ast-grep outline tests/workflow-orchestration.test.mjs --items all
rg -n -i -C 10 'failedLabel|config-resolve|state-prepare|synthesis|stage|reject|throw|failure' \
tests/workflow-orchestration.test.mjs
echo '=== build/freshness configuration references ==='
rg -n -C 3 'workflow:build|workflow:freshness|workflow-build|workflow-freshness' \
package.json MakefileLength of output: 38299
{ ok: false, stage, error: 'fixture failure' }No remaining work is required for this finding. |
2 participants
Summary
This branch compiles Dakar's ODW review workflow from strict, directly tested TypeScript modules so maintainers can change cohesive components without sacrificing the single-file runtime artefact required by ODW. It adds a deterministic, fail-closed compiler and freshness contract, preserves the installed CLI and direct-invocation interfaces, and proves parity through module, artefact, dry-run, CLI, state, and isolated live-review evidence.
Roadmap slice: (5.), completing tasks (5.1.1)–(5.3.2) in the roadmap.
ExecPlan: Compile the Dakar review workflow from typed modules.
The implementation lands all planned work: the accepted architectural decision, exact TypeScript and esbuild development contracts, typed component extraction, generated-artefact validation, synchronized documentation, and end-to-end parity proof. Review follow-up also validates trusted-base instructions and hostile workflow arguments, makes verifier labels collision-safe, exposes bounded Record retries, and property-tests task and candidate bounds. These corrections preserve the public review interface while making the typed trust boundaries explicit. No planned work is deferred.
Review walkthrough
AGENTS.mdloading, mutable-working-tree exclusion, and fail-closed Git errors.Validation
git diff --check: passed.make check-fmt: passed.make typecheck: passed, including strict TypeScript checking and an ODW dry run returningok: trueanddryRun: true.make lint: passed with 13 Markdown files reporting zero errors and every Nixie diagram valid.make test: passed with 109 tests, zero failures, and a fresh generated artefact.make workflow-freshness: passed; the generated artefact matches its TypeScript sources exactly.4f48e4a2b665a60a3d644e65630e41cdc794d18b4ec182ac32081a5916e9fe31values at the implementation milestone.skipped: trueon the same-head rerun.coderabbit review --agent: completed after each implementation milestone; the final implementation review returned zero findings before the inline follow-up addressed here.pr-creationanden-gb-oxendict-styleskill requirements.Notes
The generated ODW artefact remains committed and directly runnable. Users and installations do not require TypeScript, esbuild, or a contributor build step.
The generated-file Biome/standard-JS ignore suggestion was not applied because the repository configures neither tool and already excludes the artefact from ordinary
node --check; the ODW dry run remains its parser contract. The blanket 80% function-docstring suggestion was also not applied because no such gate exists and mechanical comments conflict with the repository's “comment why” convention; every workflow module now has the required top-level@filedocumentation instead.