Skip to content

feat: infer ops search_path from schema refs - #463

Merged
flyingrobots merged 18 commits into
mainfrom
feature/active-next-slice
Mar 30, 2026
Merged

flyingrobots merged 18 commits into
mainfrom
feature/active-next-slice

Conversation

@flyingrobots

Copy link
Copy Markdown
Owner

Summary

  • infer hardened default ops search_path values from IR metadata and schema-qualified plan refs
  • preserve manual --ops-search-path overrides while keeping schema-aware base table qualification
  • document the new default and cover the common, override, and multi-schema cases with regressions

Testing

  • node --test packages/wesley-core/test/unit/qir-translate-env.test.mjs packages/wesley-core/test/snapshots/qir-emission.test.mjs packages/wesley-cli/test/generate-ops.test.mjs
  • pnpm lint
  • node scripts/check-doc-truth.mjs
  • node scripts/check-doc-links.mjs
  • node scripts/pre-push-sanity.mjs --files $(git show --pretty='' --name-only HEAD)

Closes #439

@coderabbitai

coderabbitai Bot commented Mar 25, 2026 •

Copy link
Copy Markdown

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro

Run ID: 0c2d66ab-708b-483c-a007-ec8ae79254fa

📥 Commits

Reviewing files that changed from the base of the PR and between 7b0f3f5 and c917c3d.

📒 Files selected for processing (3)
  • CHANGELOG.md
  • CHRONICLES_OF_THE_MACHINE-KIND_VOL_00000010.jsonl
  • packages/wesley-cli/test/cert-e2e.bats

Summary by CodeRabbit

  • New Features

    • SHIPME certification now embeds and displays HOLMES summaries; HOLMES verdict is required for shipment eligibility and shown in badges.
    • SQL ops emission now infers and hardens search_path from schema references, with an explicit override option.
  • Bug Fixes

    • Treat schemas with no indexed fields as fully covered for index-coverage scoring.
  • Tests

    • Added unit and end-to-end tests covering search_path inference, schema-qualified refs, HOLMES summaries, and certification flows.
  • Documentation

    • Updated guides, policy notes, and changelog to reflect HOLMES/SHIPME and search_path behavior.
  • CI

    • Workflows updated to run HOLMES investigation and publish/detect dynamic bundle/schema outputs.

Walkthrough

Infer hardened ops search_path (and resolve baseSchema) from IR and compiled ops plans; preserve and validate schema-qualified table references through TranslateEnv and lowering; embed and gate on HOLMES summaries in SHIPME cert flow; add tests, docs, schema, workflows, and CI wiring. (29 words)

Changes

Cohort / File(s) Summary
Documentation & Chronicle
docs/guides/qir-ops.md, docs/blade.md, docs/holmes-policy-spec.md, CHRONICLES_OF_THE_MACHINE-KIND_VOL_00000010.jsonl
Document inferred hardened search_path default and changed --ops-search-path semantics; require/display HOLMES verdict in cert badges; add chronicle and policy notes.
CLI — ops generation
packages/wesley-cli/src/commands/generate-ops.mjs, packages/wesley-cli/src/commands/generate.mjs, packages/wesley-cli/test/generate-ops.test.mjs
Add exported inferOpsSchemaContext(...); compute {searchPath, baseSchema} from IR + compiled ops or explicit override; pass into emit pipeline; update help text; add tests asserting SET search_path and inference cases.
QIR lowering & resolution
packages/wesley-core/src/domain/qir/TranslateEnv.mjs, packages/wesley-core/src/domain/qir/lowerToSQL.mjs, packages/wesley-core/test/snapshots/qir-emission.test.mjs, packages/wesley-core/test/unit/qir-translate-env.test.mjs
Parse and validate schema-qualified table refs in TranslateEnv and lowerToSQL; render preserved qualified refs in emitted SQL; add positive and negative tests for malformed refs.
Certificate / HOLMES integration
packages/wesley-cli/src/commands/_cert-utils.mjs, packages/wesley-cli/src/commands/cert-badge.mjs, packages/wesley-cli/src/commands/cert-create.mjs, packages/wesley-cli/src/commands/cert-verify.mjs, packages/wesley-cli/test/cert-e2e.bats, packages/wesley-cli/test/schema-validator.test.mjs
Centralize badge/policy logic (evaluateCertificatePolicy, buildCertBadge); embed HOLMES summaries from bundle in cert-create; evaluate HOLMES ELEMENTARY gating in cert-verify; update badge rendering and add E2E/schema tests for HOLMES presence and failure modes.
CI workflows & assertions
.github/workflows/cert-shipme.yml, .github/workflows/wesley-holmes.yml, test/ci-workflows.bats
Emit Wesley bundle before HOLMES (--emit-bundle), add HOLMES investigation step writing .wesley-cache/holmes-report.*, expose wesley-generate outputs (schema, bundle_dir) for downstream jobs, adjust permissions, and add workflow assertions.
Schema & Scoring
schemas/shipme.schema.json, packages/wesley-core/src/application/Scoring.mjs, packages/wesley-core/test/unit/scoring-engine.test.mjs
Allow nullable top-level holmes in SHIPME schema; treat schemas with zero indexed fields as fully covered (calculateIndexCoverage returns 1 when total === 0); add TCI unit test.
Tests & Snapshots
packages/wesley-cli/test/generate-ops.test.mjs, packages/wesley-core/test/snapshots/qir-emission.test.mjs, packages/wesley-core/test/unit/qir-translate-env.test.mjs, packages/wesley-cli/test/cert-e2e.bats, test/ci-workflows.bats
Add/extend unit, snapshot, and E2E tests covering ops inference, qualified-ref emission/validation, HOLMES embedding/gating, workflow outputs, and scoring behavior.

Sequence Diagram(s)

sequenceDiagram
    participant CLI as "CLI: generate-ops"
    participant IR as "IR / Metadata"
    participant Compiler as "Compiled Ops (QIR)"
    participant Infer as "inferOpsSchemaContext"
    participant Emitter as "emitFunction / emitView"
    participant Lower as "lowerToSQL / TranslateEnv"

    CLI->>IR: load IR/metadata
    CLI->>Compiler: load compiled ops plans
    CLI->>Infer: inferOpsSchemaContext(IR, Compiler, explicitSearchPath?)
    Infer-->>CLI: { searchPath, baseSchema }
    CLI->>Emitter: emit artifacts with searchPath, baseSchema
    Emitter->>Lower: lower QIR (resolveTableRef, renderTableRef)
    Lower-->>Emitter: SQL with preserved/validated qualified refs
    Emitter-->>CLI: write SQL artifacts (include SET search_path)
Loading
sequenceDiagram
    participant CI as "CI workflow"
    participant Detect as "wesley-generate (detect)"
    participant Holmes as "holmes investigate"
    participant Cert as "cert-create / cert-verify / cert-badge"
    participant Repo as "GitHub (outputs / PR perms)"

    CI->>Detect: run detect -> produce schema,bundle outputs
    Detect-->>CI: set job outputs (schema, bundle_dir)
    CI->>Holmes: run investigate -> write .wesley-cache/holmes-report.json/.md
    Holmes-->>CI: report files
    CI->>Cert: cert-create reads bundle (and scores) -> build holmes summary
    Cert-->>Repo: emit SHIPME artifact and badge (may post PR comment)
    CI->>Repo: workflow uses outputs and requires PR write permissions
Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~45 minutes

Possibly related PRs

Poem

🧭 IR maps, search_path aligned,
pg_catalog first, then ops designed,
Qualified names kept true and neat,
HOLMES inspects each cert’s heartbeat,
Badges flash — CI seals the feat.

🚥 Pre-merge checks | ✅ 2 | ❌ 3

❌ Failed checks (2 warnings, 1 inconclusive)

Check name Status Explanation Resolution
Out of Scope Changes check ⚠️ Warning Changes to HOLMES integration (cert-verify, cert-create, cert-badge, workflows, schema validation, E2E tests) are out of scope for issue #439 which targets ops search_path inference only. Separate HOLMES certification pipeline changes (workflows, cert-*.mjs, E2E tests, schema updates) into a distinct PR; keep this PR focused exclusively on ops search_path inference and its direct test coverage.
Docstring Coverage ⚠️ Warning Docstring coverage is 10.81% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
Description check ❓ Inconclusive Description covers Summary, Testing sections with command references; however, omits Why, Changes, Risk, Backout, and Merge Strategy sections from the template. Expand description to include Why (rationale and alternatives), Changes (bulleted surgical list), Risk (user/CI risk and mitigations), Backout (revert safety), and Merge Strategy sections per template.
✅ Passed checks (2 passed)
Check name Status Explanation
Title check ✅ Passed Title accurately summarizes the primary change: inferring ops search_path from schema references instead of requiring manual specification.
Linked Issues check ✅ Passed Changes comprehensively address #439 objectives: inferring search_path from IR/schema refs [docs/guides/qir-ops.md, generate-ops.mjs], preserving manual overrides [--ops-search-path flag], and adding regression coverage for common/override/multi-schema cases [generate-ops.test.mjs, snapshots, unit tests].

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch feature/active-next-slice

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@coderabbitai coderabbitai Bot added enhancement New feature or request feature New feature or request labels Mar 25, 2026
@github-actions

github-actions Bot commented Mar 25, 2026 •

Copy link
Copy Markdown

🔍 The Case of Pull Request #463

🕵️ SHA-lock HOLMES's Investigation (click to expand)

🕵️ SHA-lock HOLMES Investigation

  • Generated: 2026-03-27T01:15:31.390Z
  • Commit SHA: 6336ced
  • Bundle Version: 2.0.0

⚠️ Evidence valid only for commit 6336ced

🔍 Executive Deduction

"Watson, after careful examination of the evidence, I deduce..."

Weighted Completion: ██████████ 100.0%
Scores: SCS 100.0% · TCI 65.0% · MRI 0.0%
Verification Status: 91 claims verified
Citation Quality: 91 exact · 0 whole-file · 0 coarse
Evidence Trust: strong
Ship Verdict: REQUIRES INVESTIGATION

🧩 SCS Breakdown

Component Score Coverage
Sql 100.0% 154.00/154.00
Types 0.0% —
Validation 0.0% —
Tests 100.0% 154.00/154.00

🧪 TCI Breakdown

Component Score Coverage Note
Unit Constraints 100.0% 104/104 N/A
Unit Rls 0.0% — N/A
Integration Relations 100.0% 3/3 N/A
E2e Ops N/A — Query operation test tracking not yet implemented

⚠️ MRI Breakdown

Component Risk Share Points Count
Drops 0% 0 0
Renames Without Uid 0% 0 0
Add Not Null Without Default 0% 0 0
Non Concurrent Indexes 0% 0 0

📊 The Weight of Evidence

"Observe, Watson, how not all features carry equal importance..."

Element Weight Status Evidence Strength Deduction
col:User.id 5 ⚠️ Whole-file/mixed SQL only out/schema.sql:3-3@6336ced exact Incomplete
col:User.email 5 ⚠️ Whole-file/mixed SQL only out/schema.sql:5-5@6336ced exact Incomplete
col:User.password_hash 5 ⚠️ Whole-file/mixed SQL only out/schema.sql:7-7@6336ced exact Incomplete
col:User.full_name 5 ⚠️ Whole-file/mixed SQL only out/schema.sql:8-8@6336ced exact Incomplete
col:User.phone 5 ⚠️ Whole-file/mixed SQL only out/schema.sql:9-9@6336ced exact Incomplete
col:User.email_verified 5 ⚠️ Whole-file/mixed SQL only out/schema.sql:10-10@6336ced exact Incomplete
col:User.created_at 5 ⚠️ Whole-file/mixed SQL only out/schema.sql:11-11@6336ced exact Incomplete
col:Product.id 5 ⚠️ Whole-file/mixed SQL only out/schema.sql:14-14@6336ced exact Incomplete
col:Product.sku 5 ⚠️ Whole-file/mixed SQL only out/schema.sql:16-16@6336ced exact Incomplete
col:Product.name 5 ⚠️ Whole-file/mixed SQL only out/schema.sql:18-18@6336ced exact Incomplete
col:Product.slug 5 ⚠️ Whole-file/mixed SQL only out/schema.sql:19-19@6336ced exact Incomplete
col:Product.price_cents 5 ⚠️ Whole-file/mixed SQL only out/schema.sql:21-21@6336ced exact Incomplete
col:Product.stock_quantity 5 ⚠️ Whole-file/mixed SQL only out/schema.sql:22-22@6336ced exact Incomplete
col:Product.published 5 ⚠️ Whole-file/mixed SQL only out/schema.sql:23-23@6336ced exact Incomplete
col:Product.created_at 5 ⚠️ Whole-file/mixed SQL only out/schema.sql:24-24@6336ced exact Incomplete
col:Order.id 5 ⚠️ Whole-file/mixed SQL only out/schema.sql:27-27@6336ced exact Incomplete
col:Order.order_number 5 ⚠️ Whole-file/mixed SQL only out/schema.sql:29-29@6336ced exact Incomplete
col:Order.user_id 5 ⚠️ Whole-file/mixed SQL only out/schema.sql:31-31@6336ced exact Incomplete
col:Order.status 5 ⚠️ Whole-file/mixed SQL only out/schema.sql:33-33@6336ced exact Incomplete
col:Order.total_cents 5 ⚠️ Whole-file/mixed SQL only out/schema.sql:34-34@6336ced exact Incomplete
col:Order.payment_intent_id 5 ⚠️ Whole-file/mixed SQL only out/schema.sql:35-35@6336ced exact Incomplete
col:Order.paid_at 5 ⚠️ Whole-file/mixed SQL only out/schema.sql:37-37@6336ced exact Incomplete
col:Order.created_at 5 ⚠️ Whole-file/mixed SQL only out/schema.sql:38-38@6336ced exact Incomplete
col:OrderItem.id 5 ⚠️ Whole-file/mixed SQL only out/schema.sql:41-41@6336ced exact Incomplete
col:OrderItem.order_id 5 ⚠️ Whole-file/mixed SQL only out/schema.sql:43-43@6336ced exact Incomplete
col:OrderItem.product_id 5 ⚠️ Whole-file/mixed SQL only out/schema.sql:45-45@6336ced exact Incomplete
col:OrderItem.quantity 5 ⚠️ Whole-file/mixed SQL only out/schema.sql:47-47@6336ced exact Incomplete
col:OrderItem.unit_price_cents 5 ⚠️ Whole-file/mixed SQL only out/schema.sql:48-48@6336ced exact Incomplete
tbl:User 5 ⛔ Missing out/tests.sql:11-11@6336ced exact Missing
tbl:Product 5 ⛔ Missing out/tests.sql:57-57@6336ced exact Missing
tbl:Order 5 ⛔ Missing out/tests.sql:109-109@6336ced exact Missing
tbl:OrderItem 5 ⛔ Missing out/tests.sql:161-161@6336ced exact Missing
col:User.id.pk 5 ⛔ Missing out/tests.sql:203-203@6336ced exact Missing
col:User.email.unique 5 ⛔ Missing out/tests.sql:204-204@6336ced exact Missing
col:Product.id.pk 5 ⛔ Missing out/tests.sql:207-207@6336ced exact Missing
col:Product.sku.unique 5 ⛔ Missing out/tests.sql:208-208@6336ced exact Missing
col:Product.slug.unique 5 ⛔ Missing out/tests.sql:209-209@6336ced exact Missing
col:Order.id.pk 5 ⛔ Missing out/tests.sql:212-212@6336ced exact Missing
col:Order.order_number.unique 5 ⛔ Missing out/tests.sql:213-213@6336ced exact Missing
col:Order.user_id.fk 5 ⛔ Missing out/tests.sql:214-214@6336ced exact Missing
col:Order.payment_intent_id.unique 5 ⛔ Missing out/tests.sql:215-215@6336ced exact Missing
col:OrderItem.id.pk 5 ⛔ Missing out/tests.sql:218-218@6336ced exact Missing
col:OrderItem.order_id.fk 5 ⛔ Missing out/tests.sql:219-219@6336ced exact Missing
col:OrderItem.product_id.fk 5 ⛔ Missing out/tests.sql:220-220@6336ced exact Missing
col:User.id.default 5 ⛔ Missing out/tests.sql:230-230@6336ced exact Missing
col:User.email_verified.default 5 ⛔ Missing out/tests.sql:234-234@6336ced exact Missing
col:User.created_at.default 5 ⛔ Missing out/tests.sql:238-238@6336ced exact Missing
col:Product.id.default 5 ⛔ Missing out/tests.sql:243-243@6336ced exact Missing
col:Product.stock_quantity.default 5 ⛔ Missing out/tests.sql:247-247@6336ced exact Missing
col:Product.published.default 5 ⛔ Missing out/tests.sql:251-251@6336ced exact Missing
col:Product.created_at.default 5 ⛔ Missing out/tests.sql:255-255@6336ced exact Missing
col:Order.id.default 5 ⛔ Missing out/tests.sql:260-260@6336ced exact Missing
col:Order.created_at.default 5 ⛔ Missing out/tests.sql:264-264@6336ced exact Missing
col:OrderItem.id.default 5 ⛔ Missing out/tests.sql:269-269@6336ced exact Missing
col:User.email.index 5 ⛔ Missing out/tests.sql:281-281@6336ced exact Missing
col:Product.sku.index 5 ⛔ Missing out/tests.sql:284-284@6336ced exact Missing
col:Product.slug.index 5 ⛔ Missing out/tests.sql:286-286@6336ced exact Missing
col:Product.published.index 5 ⛔ Missing out/tests.sql:288-288@6336ced exact Missing
col:Order.order_number.index 5 ⛔ Missing out/tests.sql:291-291@6336ced exact Missing
col:Order.user_id.index 5 ⛔ Missing out/tests.sql:293-293@6336ced exact Missing
col:Order.payment_intent_id.index 5 ⛔ Missing out/tests.sql:295-295@6336ced exact Missing
col:OrderItem.order_id.index 5 ⛔ Missing out/tests.sql:298-298@6336ced exact Missing
col:OrderItem.product_id.index 5 ⛔ Missing out/tests.sql:300-300@6336ced exact Missing

🚪 Security & Performance Gates

"Elementary security measures, Watson..."

Gate Status Evidence Holmes's Ruling
Migration Risk ✅ MRI: 0.0% "Trivial risk"
Test Coverage ⚠️ TCI: 65.0% "Insufficient coverage"
Sensitive Fields ⛔ 1 fields "1 EXPOSED!"
Evidence Quality ✅ 91 exact · 0 whole-file · 0 coarse "All 91 citations resolve to exact line spans."

📋 The Verdict

⚠️ REQUIRES FURTHER INVESTIGATION
"Some clues remain unclear. Address the noted issues."

Signed and sealed,

  • S. Holmes, Consulting Detective

[END OF INVESTIGATION FOR COMMIT 6336ced]

🧵 Command Run

  • Run ID: run-mn87my98-y3rovn
  • Transmutation: holmes-investigate
  • Command: investigate
  • Status: completed
  • Ledger: /home/runner/work/wesley/wesley/test/fixtures/examples/.wesley-cache/ledger

🩺 Dr. WATSON's Verification (click to expand)

🩺 Dr. Watson's Independent Verification Report

Medical Examination of Evidence

  • Examination Date: 2026-03-27T01:16:31.616Z
  • Patient SHA: 6336ced

🔬 Citation Verification

"Let me examine each piece of evidence independently..."

  • Citations Examined: 91
  • Verified: 0 ✅
  • Failed: 0 ❌
  • Unable to Verify: 91
  • Exact Subrange Citations: 0
  • Whole-file Citations: 0
  • Coarse Citations: 0
  • Evidence Trust: missing
  • Trust Note: No evidence citations were available for trust analysis.

Verification Rate: 0.0%

📊 Mathematical Verification

"I shall recalculate Holmes's arithmetic..."

Holmes claimed SCS: 100.0%
Watson calculates: 0.0%
Difference: ⚠️ Significant

🔍 Consistency Analysis

"Checking for contradictions in Holmes's deductions..."

⚠️ Sensitive field col:User.password_hash lacks test coverage

🩺 Dr. Watson's Medical Opinion

VERIFICATION: CONCERNS NOTED ⚠️

"While Holmes's methods are generally sound, I have noted some"
"discrepancies that warrant further investigation. No evidence citations were available for trust analysis."

Respectfully submitted,

  • Dr. J. Watson, M.D.
    Medical Examiner & Verification Specialist

🧵 Command Run

  • Run ID: run-mn87nlkm-z5byid
  • Transmutation: watson-verify
  • Command: verify
  • Status: completed
  • Ledger: /home/runner/work/wesley/wesley/test/fixtures/examples/.wesley-cache/ledger

🔮 Professor MORIARTY's Predictions (click to expand)

🧠 Professor Moriarty's Temporal Predictions

The Mathematics of Inevitability

  • Analysis Date: 2026-03-27T01:17:16.972Z

🔮 Current State

SCS: ██████████ 100.0%
TCI: ███████░░░ 65.0%
MRI: 0.0% risk
Evidence Trust: strong

📈 Velocity Analysis

SCS Velocity: +0.00%/day
Git Activity (window): 24h · commits 4 (0 relevant) · ~4.00 commits/day
↳ Magnitude: ~0 relevant LOC/day across ~0.0 files/day
Activity Index: 7 / 100 (PR 0, Window 17)
Blended Velocity: +0.04%/day
Commit Size Burstiness: 0 / 100 (higher = more uneven commit sizes)
⚠️ PLATEAU DETECTED - Low SCS movement and low recent Git activity.

⏰ Completion Predictions

ETA: Cannot predict (insufficient velocity)

"At current velocity, completion is... improbable."

🧪 Readiness EXPLAIN

  • SCS ≥ 80% → PASS ✅ (actual 100.0%)
  • TCI ≥ 70% → FAIL ❌ (actual 65.0%)
  • MRI ≤ 40% → PASS ✅ (actual 0.0%)
  • CI Stability ≥ 90% (branch main) → PASS ✅ (actual 91% over ~168h)
  • Evidence Trust ≥ moderate → PASS ✅ (actual strong) — All 91 citations resolve to exact line spans.
  • Delivery context (last 168h): 12 issues closed · 0 PRs merged (informational, not gating)

Signals blend: SCS velocity (70%) + Git activity (30%, branch-first). Activity only suppresses false plateaus; it never inflates readiness.

📊 Historical Trajectory

03-27: ██████████ 100.0%
03-27: ██████████ 100.0%
03-27: ██████████ 100.0%

"Every problem becomes elementary when reduced to mathematics"
— Professor Moriarty

🧵 Command Run

  • Run ID: run-mn87okkq-5n50mx
  • Transmutation: moriarty-predict
  • Command: predict
  • Status: completed
  • Ledger: /home/runner/work/wesley/wesley/test/fixtures/examples/.wesley-cache/ledger

Machine-readable reports: holmes-report.json · watson-report.json · moriarty-report.json (see workflow artifacts).


Filed at 221B Repository Street

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 47ce1162c5

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread packages/wesley-cli/src/commands/generate-ops.mjs Outdated

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 5

🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@CHRONICLES_OF_THE_MACHINE-KIND_VOL_00000010.jsonl`:
- Line 75: The timestamp field in the JSONL entry uses fractional seconds
("timestamp":"2026-03-25T07:20:33.312Z") but the chronicle contract requires UTC
ISO 8601 timestamps with second precision; update the timestamp value to remove
the fractional seconds ("2026-03-25T07:20:33Z") by editing the "timestamp" field
in the entry, and run the chronicle/CI timestamp validation (or your local JSONL
validator) to ensure no other entries in
CHRONICLES_OF_THE_MACHINE-KIND_VOL_00000010.jsonl contain fractional seconds.

In `@packages/wesley-cli/src/commands/generate-ops.mjs`:
- Around line 424-425: The current logic removes the inferred base schema when
sanitizeSchemaName(ir?.metadata?.schemaName, normalizedSchema) returns the
ops/target schema, causing baseSchema to fall back to 'public' and emit
unqualified refs; change the behavior so that when computing irSchema (using
sanitizeSchemaName) you still add the original ir.metadata.schemaName to
inferred (or preserve the inferred schema) if it equals the target/ops schema so
generated ops use the real schema, not 'public'; update the code paths around
sanitizeSchemaName, the inferred.add(irSchema) call, and the baseSchema fallback
logic (the block that sets baseSchema to 'public') so they preserve and honor
the inferred base schema when it matches targetSchema.
- Around line 417-445: The batch-level baseSchema decision uses the global
planSchemaRefs set, causing one qualified table anywhere to null out baseSchema
for all ops; instead, compute qualified refs per compiled entry by calling
collectPlanSchemaRefs(entry?.plan, localSet) for each entry and use that
per-entry result to decide baseSchema (or null) for that specific op; update the
logic that sets baseSchema/searchPath (references: compiledOps,
collectPlanSchemaRefs, planSchemaRefs, inferredSchemas, baseSchema,
hasQualifiedTableRefs, searchPath, explicit, dedupeSearchPath,
sanitizeSchemaName, ir, normalizedSchema) so the global inferredSchemas remain
for search_path fallback but baseSchema is only nulled for ops whose own plan
contains qualified table refs.

In `@packages/wesley-core/src/domain/qir/lowerToSQL.mjs`:
- Around line 146-149: The splitQualifiedTableRef function should reject
malformed qualified refs instead of collapsing empty segments; remove the
filter(Boolean) behavior and change the logic in splitQualifiedTableRef to split
on '.', trim each part, and return the two-part array only if the original split
yielded exactly two segments and neither trimmed segment is an empty
string—otherwise return null (i.e., detect inputs like "app..orders" or
".orders" as invalid). Ensure you update the check around parts length and
emptiness in splitQualifiedTableRef accordingly.

In `@packages/wesley-core/src/domain/qir/TranslateEnv.mjs`:
- Around line 265-277: splitSchemaQualifiedRef currently filters out empty
segments which causes malformed refs like "app..Product" to be accepted; change
the logic so you first split without filtering and only accept exactly two
non-empty segments: in function splitSchemaQualifiedRef, remove the
.filter(Boolean) behavior and instead check that parts.length === 2 and both
parts[0] and parts[1] are non-empty (trimmed) before returning {schemaName:
parts[0], baseRef: parts[1]}; if that validation fails, return { schemaName:
null, baseRef: ref } to ensure malformed schema-qualified refs are rejected
rather than normalized.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro

Run ID: 1f0149d2-0e49-4c83-a337-de95de6ea6c7

📥 Commits

Reviewing files that changed from the base of the PR and between 5479415 and 47ce116.

📒 Files selected for processing (9)
  • CHRONICLES_OF_THE_MACHINE-KIND_VOL_00000010.jsonl
  • docs/guides/qir-ops.md
  • packages/wesley-cli/src/commands/generate-ops.mjs
  • packages/wesley-cli/src/commands/generate.mjs
  • packages/wesley-cli/test/generate-ops.test.mjs
  • packages/wesley-core/src/domain/qir/TranslateEnv.mjs
  • packages/wesley-core/src/domain/qir/lowerToSQL.mjs
  • packages/wesley-core/test/snapshots/qir-emission.test.mjs
  • packages/wesley-core/test/unit/qir-translate-env.test.mjs

Comment thread CHRONICLES_OF_THE_MACHINE-KIND_VOL_00000010.jsonl
Comment thread packages/wesley-cli/src/commands/generate-ops.mjs Outdated
Comment thread packages/wesley-cli/src/commands/generate-ops.mjs Outdated
Comment thread packages/wesley-core/src/domain/qir/lowerToSQL.mjs
Comment thread packages/wesley-core/src/domain/qir/TranslateEnv.mjs

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

♻️ Duplicate comments (2)
packages/wesley-cli/src/commands/generate-ops.mjs (2)

153-163: ⚠️ Potential issue | 🟠 Major

Infer the hardened schema context per op, not once for the whole batch.

Line 153 computes a single schemaContext from all orderedOps, but emission is still per-entry and can skip failures. With --ops-allow-errors, a bad op can still widen SET search_path or change baseSchema for the ops that do get written, and even without skips every function/view inherits schemas from unrelated ops in the batch. Move this inference into the per-op emission path so each artifact only gets the schemas it actually needs.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@packages/wesley-cli/src/commands/generate-ops.mjs` around lines 153 - 163,
The code currently calls inferOpsSchemaContext once for the whole batch
(schemaContext used to set setSearchPath/baseSchema) before calling
emitOpArtifacts for orderedOps; change this so you infer the hardened schema
context per individual op inside the per-op emission loop: for each entry in
orderedOps call inferOpsSchemaContext with that single op (or a minimal
compiledOps containing only that op) and then call emitOpArtifacts (or the
per-op emitter) passing schemaContext.searchPath and schemaContext.baseSchema
for that one artifact; ensure emit logic uses those per-op values so a failing
or unrelated op cannot widen SET search_path or change baseSchema for other
emitted artifacts (respecting --ops-allow-errors behavior).

610-612: ⚠️ Potential issue | 🟠 Major

Keep schema inference aligned with the core qualified-ref parser.

Line 611 still uses filter(Boolean), so app..orders is inferred as schema app here even though packages/wesley-core/src/domain/qir/lowerToSQL.mjs and packages/wesley-core/src/domain/qir/TranslateEnv.mjs now reject that shape. That lets a malformed op contaminate inferred search_path/baseSchema before emission. Parse exactly two non-empty segments here too.

Suggested fix
 function extractSchemaName(tableRef) {
-  const parts = String(tableRef).split('.').map((part) => part.trim()).filter(Boolean);
-  return parts.length === 2 ? parts[0] : null;
+  const parts = String(tableRef).split('.').map((part) => part.trim());
+  if (parts.length !== 2) return null;
+  if (!parts[0] || !parts[1]) return null;
+  return parts[0];
 }
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@packages/wesley-cli/src/commands/generate-ops.mjs` around lines 610 - 612,
The extractSchemaName function currently uses
String(tableRef).split('.').map(...).filter(Boolean) which accepts inputs like
"app..orders"; change it to parse and require exactly two non-empty segments
without using filter(Boolean): split on '.', trim parts, then verify
parts.length === 2 and both parts[0] and parts[1] are non-empty before returning
parts[0]; otherwise return null so the behavior matches the core qualified-ref
parser (function extractSchemaName).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Duplicate comments:
In `@packages/wesley-cli/src/commands/generate-ops.mjs`:
- Around line 153-163: The code currently calls inferOpsSchemaContext once for
the whole batch (schemaContext used to set setSearchPath/baseSchema) before
calling emitOpArtifacts for orderedOps; change this so you infer the hardened
schema context per individual op inside the per-op emission loop: for each entry
in orderedOps call inferOpsSchemaContext with that single op (or a minimal
compiledOps containing only that op) and then call emitOpArtifacts (or the
per-op emitter) passing schemaContext.searchPath and schemaContext.baseSchema
for that one artifact; ensure emit logic uses those per-op values so a failing
or unrelated op cannot widen SET search_path or change baseSchema for other
emitted artifacts (respecting --ops-allow-errors behavior).
- Around line 610-612: The extractSchemaName function currently uses
String(tableRef).split('.').map(...).filter(Boolean) which accepts inputs like
"app..orders"; change it to parse and require exactly two non-empty segments
without using filter(Boolean): split on '.', trim parts, then verify
parts.length === 2 and both parts[0] and parts[1] are non-empty before returning
parts[0]; otherwise return null so the behavior matches the core qualified-ref
parser (function extractSchemaName).

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro

Run ID: 409cf87a-1c29-41a8-8d58-7b1748445e8c

📥 Commits

Reviewing files that changed from the base of the PR and between 47ce116 and d9099d1.

📒 Files selected for processing (6)
  • packages/wesley-cli/src/commands/generate-ops.mjs
  • packages/wesley-cli/test/generate-ops.test.mjs
  • packages/wesley-core/src/domain/qir/TranslateEnv.mjs
  • packages/wesley-core/src/domain/qir/lowerToSQL.mjs
  • packages/wesley-core/test/snapshots/qir-emission.test.mjs
  • packages/wesley-core/test/unit/qir-translate-env.test.mjs

coderabbitai[bot]
coderabbitai Bot previously approved these changes Mar 25, 2026

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 5

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (2)
.github/workflows/wesley-holmes.yml (2)

149-182: ⚠️ Potential issue | 🔴 Critical

Missing job dependency: watson-verify cannot access needs.wesley-generate.

Line 152 declares needs: holmes-investigate only, but lines 169-170 and 181-182 reference needs.wesley-generate.outputs.*. GitHub Actions only exposes outputs from jobs explicitly in the needs array — this will fail at runtime.

🔧 Fix: Add `wesley-generate` to the needs array
   watson-verify:
     name: "🩺 WATSON Verification"
     runs-on: ubuntu-latest
-    needs: holmes-investigate
+    needs: [wesley-generate, holmes-investigate]
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In @.github/workflows/wesley-holmes.yml around lines 149 - 182, The
watson-verify job references needs.wesley-generate.outputs.* but its needs array
only lists holmes-investigate; update the watson-verify job definition (job
name: watson-verify) to include wesley-generate in the needs array (e.g., needs:
[holmes-investigate, wesley-generate]) so the referenced outputs
(needs.wesley-generate.outputs.bundle_dir and
needs.wesley-generate.outputs.schema) are available at runtime.

191-338: ⚠️ Potential issue | 🔴 Critical

Same issue: moriarty-predict cannot access needs.wesley-generate.

Line 194 declares needs: watson-verify only, but lines 213-214, 233, and 337-338 reference needs.wesley-generate.outputs.*. Will fail identically.

🔧 Fix: Add `wesley-generate` to the needs array
   moriarty-predict:
     name: "🔮 MORIARTY Predictions"
     runs-on: ubuntu-latest
-    needs: watson-verify
+    needs: [wesley-generate, watson-verify]
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In @.github/workflows/wesley-holmes.yml around lines 191 - 338, The
moriarty-predict job is missing the required dependency on wesley-generate so
references like needs.wesley-generate.outputs.bundle_dir and
needs.wesley-generate.outputs.schema will be undefined; update the
moriarty-predict job's needs list (job name: moriarty-predict) to include
wesley-generate alongside watson-verify so the outputs used in the HOLMES_SETUP
step and the run-holmes-command step resolve correctly.
🤖 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/workflows/cert-shipme.yml:
- Around line 22-25: Remove the unnecessary and overly-broad permission grant by
deleting the "pull-requests: write" permission entry from the workflow
permissions block (the YAML key "pull-requests: write"); the PR comment step
only calls Issues API methods
(github.rest.issues.listComments/updateComment/createComment) which are covered
by "issues: write", so remove that key to restore least-privilege.

In `@packages/wesley-cli/src/commands/_cert-utils.mjs`:
- Around line 27-35: The badge currently only uses realm.verdict to decide
PASS/FAIL in buildCertBadge; update it to treat a counterfactual hard-fail as a
fail by computing a counterfactualFail flag (e.g., json?.counterfactual?.gate
=== 'fail') and change okRealm to require realm.verdict === 'PASS' &&
!counterfactualFail so the badge shows FAIL if counterfactual.gate === 'fail';
keep the rest (sha, holmesVerdict, parts) unchanged so HOLMES and sha are still
included.

In `@packages/wesley-cli/src/commands/cert-create.mjs`:
- Around line 283-290: Remove the unnecessary scores guard and pass the original
bundle into the Holmes constructor inside buildShipmeHolmesSummary: stop
returning null when scores is missing, call new Holmes(bundle) (not new Holmes({
...bundle, scores })) and then call investigationData(); ensure the function
still checks for bundle?.evidence but does not require scores so cert.holmes is
populated in bundle-only environments that the Holmes.investigationData() path
supports.

In `@schemas/shipme.schema.json`:
- Around line 75-77: Change the three HOLMES counter properties so they require
integers instead of arbitrary numbers: update verificationCount, gateFailures,
and gateWarnings in the schema to use "type": "integer" (preserving "minimum":
0) so fractional values (e.g., 0.5) are rejected and the contract enforces
whole-number counts.

In `@test/ci-workflows.bats`:
- Around line 77-93: The test checks for consumers of
needs.wesley-generate.outputs.bundle_dir but doesn't verify that the
wesley-generate job actually exposes bundle_dir by mapping
steps.detect.outputs.bundle_dir into its job outputs; update the test to assert
that the producer job's outputs includes a mapping like "outputs: bundle_dir:
${{ steps.detect.outputs.bundle_dir }}" (i.e., add an assertion that grep finds
"outputs.bundle_dir" or the exact "steps.detect.outputs.bundle_dir" within the
job outputs block) so you prove that steps.detect.outputs.bundle_dir is exported
as needs.wesley-generate.outputs.bundle_dir in wesley-holmes.yml.

---

Outside diff comments:
In @.github/workflows/wesley-holmes.yml:
- Around line 149-182: The watson-verify job references
needs.wesley-generate.outputs.* but its needs array only lists
holmes-investigate; update the watson-verify job definition (job name:
watson-verify) to include wesley-generate in the needs array (e.g., needs:
[holmes-investigate, wesley-generate]) so the referenced outputs
(needs.wesley-generate.outputs.bundle_dir and
needs.wesley-generate.outputs.schema) are available at runtime.
- Around line 191-338: The moriarty-predict job is missing the required
dependency on wesley-generate so references like
needs.wesley-generate.outputs.bundle_dir and
needs.wesley-generate.outputs.schema will be undefined; update the
moriarty-predict job's needs list (job name: moriarty-predict) to include
wesley-generate alongside watson-verify so the outputs used in the HOLMES_SETUP
step and the run-holmes-command step resolve correctly.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro

Run ID: e9b441ad-8f4a-419c-bb5f-a6aa4542c8ec

📥 Commits

Reviewing files that changed from the base of the PR and between d9099d1 and 70e9f42.

📒 Files selected for processing (11)
  • .github/workflows/cert-shipme.yml
  • .github/workflows/wesley-holmes.yml
  • CHRONICLES_OF_THE_MACHINE-KIND_VOL_00000010.jsonl
  • packages/wesley-cli/src/commands/_cert-utils.mjs
  • packages/wesley-cli/src/commands/cert-badge.mjs
  • packages/wesley-cli/src/commands/cert-create.mjs
  • packages/wesley-cli/src/commands/cert-verify.mjs
  • packages/wesley-cli/test/cert-e2e.bats
  • packages/wesley-cli/test/schema-validator.test.mjs
  • schemas/shipme.schema.json
  • test/ci-workflows.bats

Comment thread .github/workflows/cert-shipme.yml
Comment thread packages/wesley-cli/src/commands/_cert-utils.mjs Outdated
Comment thread packages/wesley-cli/src/commands/cert-create.mjs Outdated
Comment thread schemas/shipme.schema.json Outdated
Comment thread test/ci-workflows.bats

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@packages/wesley-cli/src/commands/cert-verify.mjs`:
- Around line 54-58: The output currently sets holmesVerdict from raw
json?.holmes?.shipVerdict which can disagree with the normalized policy fields
(holmesPassed, reasons); replace that expression with the normalized value
policy.holmesVerdict so holmesVerdict, holmesPassed and reasons are consistent
(update the object field assignment for holmesVerdict to use
policy.holmesVerdict and keep holmes: json?.holmes || null as the raw payload).
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro

Run ID: 05e458fe-5b51-4f1a-9c21-fff08ab5f713

📥 Commits

Reviewing files that changed from the base of the PR and between 70e9f42 and ab88bfb.

📒 Files selected for processing (6)
  • CHRONICLES_OF_THE_MACHINE-KIND_VOL_00000010.jsonl
  • docs/blade.md
  • docs/holmes-policy-spec.md
  • packages/wesley-cli/src/commands/_cert-utils.mjs
  • packages/wesley-cli/src/commands/cert-verify.mjs
  • packages/wesley-cli/test/cert-e2e.bats

Comment thread packages/wesley-cli/src/commands/cert-verify.mjs Outdated
@flyingrobots

Copy link
Copy Markdown
Owner Author

@coderabbitai Thanks. I checked actual repo behavior before narrowing this permission. In run 23530391194, the Create/Update PR Comment step logged Could not post SHIPME badge to PR: Resource not accessible by integration when the workflow token only had issues: write, even though the script only uses Issues API methods. The local regression guard now intentionally pins pull-requests: write in test/ci-workflows.bats because, in this repo and trigger shape, removing it regressed the PR badge comment path. I am keeping the grant until we have a reproducible repo-local proof that the narrower scope works end-to-end.

@coderabbitai

coderabbitai Bot commented Mar 25, 2026

Copy link
Copy Markdown

Tip

For best results, initiate chat on the files or code changes.

@flyingrobots: Understood — run 23530391194 is solid empirical evidence, and the pinned regression guard in test/ci-workflows.bats is exactly the right way to document and protect this.

This is a known GitHub Actions quirk: GitHub's token permission model does not always map 1:1 to the underlying REST API endpoint. Even though PR comments go through the Issues API (/repos/.../issues/{n}/comments), certain trigger shapes (e.g. pull_request_target, specific repo-level permission policies, or organization settings) can cause the runtime check to gate on pull-requests: write rather than issues: write. The error message "Resource not accessible by integration" is the canonical signal for this — it's a permission boundary enforced by GitHub's integration layer, not the API surface itself.

Your stated exit criterion (a reproducible repo-local proof that the narrower scope works end-to-end) is the right bar. The current state is well-justified. No further action needed here.


✏️ Learnings added
Learnt from: flyingrobots
URL: https://github.com/flyingrobots/wesley/pull/463

Timestamp: 2026-03-25T13:51:01.105Z
Learning: In `flyingrobots/wesley`, `.github/workflows/cert-shipme.yml` requires `pull-requests: write` (not just `issues: write`) for the "Create/Update PR Comment" step to succeed. Run `23530391194` confirmed that `issues: write` alone caused "Resource not accessible by integration" even though the script uses the GitHub Issues REST API. This is a known GitHub Actions quirk where certain trigger shapes gate PR comment creation on `pull-requests: write` at the integration layer. The regression guard in `test/ci-workflows.bats` intentionally asserts this permission is present. Do not flag this as an unnecessary permission widening without first checking this empirical context.

Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.


🧠 Learnings used
Learnt from: CR
Repo: flyingrobots/wesley PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-02-12T15:12:26.547Z
Learning: Applies to .github/workflows/**/*.{yml,yaml} : Use least privilege in workflows; do not widen permissions or add secrets without approval

Learnt from: CR
Repo: flyingrobots/wesley PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-02-12T15:12:26.547Z
Learning: Applies to .github/workflows/**/*.{yml,yaml} : Only touch GitHub Actions workflows when fixing a deterministic break or reducing obvious runtime/duplication, and provide a revert path in the PR description

Learnt from: CR
Repo: flyingrobots/wesley PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-02-12T19:09:57.726Z
Learning: Applies to .github/workflows/cli-*.{yml,yaml} : Dedicated CLI workflows (e.g., cli-tests.yml, cli-quick.yml) own CLI end-to-end/bats checks; avoid duplicating them in main CI without approval

Learnt from: CR
Repo: flyingrobots/wesley PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-02-12T15:12:26.547Z
Learning: One-topic PRs with a tight diff; CI should pass unless the PR specifically repairs CI

Learnt from: flyingrobots
Repo: flyingrobots/wesley PR: 400
File: packages/wesley-cli/src/framework/irToSchema.mjs:0-0
Timestamp: 2026-03-08T13:56:43.735Z
Learning: In `flyingrobots/wesley`, `packages/wesley-cli/src/framework/irToSchema.mjs` was rewritten in commit 042fd61 (PR `#400`). The rewritten adapter now expects the IR to use `t.fields[]` (not `t.columns[]`) with structured `FieldType` objects (`f.type.base`, `f.type.isList`, `f.nullable`) rather than plain PostgreSQL type strings. Table directives are passed through via `t.directives || {}`. Field directives are translated from structured IR flags (`pk`, `fk`, `unique`, `default`, `index`) to domain `@`-prefixed format by `buildFieldDirectives`. The prior learning about `t.columns` with plain PG strings (canonical runtime IR) is now superseded for this adapter file.

Learnt from: flyingrobots
Repo: flyingrobots/wesley PR: 400
File: packages/wesley-cli/src/framework/irToSchema.mjs:0-0
Timestamp: 2026-03-08T13:56:14.415Z
Learning: In the `flyingrobots/wesley` repository, `packages/wesley-cli/src/framework/irToSchema.mjs` (post-commit 042fd61) reads the CURRENT RUNTIME IR which uses structured `FieldType` objects. The IR now has `ir.tables[].fields[]` (NOT `columns[]`), where each field has `f.type.base` (a GraphQL scalar name like `"ID"`, `"String"`, `"DateTime"`, `"JSON"`), `f.type.isList` (boolean), and `f.nullable` (boolean). There is NO PostgreSQL-to-GraphQL mapping (`PG_TO_GQL`) in this file anymore. The adapter is a straightforward pass-through from the structured IR to core `Schema`/`Table`/`Field` objects. The `WesleyIR.schema.ts` shape with `{ base, isList, listItemNullable }` IS the current live runtime IR shape as of this migration.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 2

🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@packages/wesley-cli/test/cert-e2e.bats`:
- Around line 136-187: The fixture .wesley-cache/bundle.json used in the test
(the bundle JSON created in the cert-e2e.bats diff) is missing the
bundle.testResults payload so Holmes’ verification/execution/conclusion logic
isn’t exercised; update the fixture JSON to include a realistic "testResults"
object under the top-level bundle (e.g., alongside "scores" and "evidence") that
contains representative test suites, test cases and pass/fail outcomes (matching
the files referenced in "evidence.schema.tests" and "evidence.schema.sql") so
that code paths which read bundle.testResults are executed during the e2e tests
and policy-facing summaries are validated.
- Around line 125-223: The test defines duplicate HOLMES fixture builders (both
blocks inside create_holmes_summary_inputs) which will diverge; refactor by
extracting a single parameterized helper (e.g., create_holmes_fixture or a
reusable buildBundleJSON function) and call it twice with different parameters
instead of duplicating the JSON and file stubs inside
create_holmes_summary_inputs; update the function to write schema.sql, tests.sql
and then call the helper to render .wesley-cache/bundle.json and
.wesley-cache/scores.json based on passed parameters
(sha/timestamp/scores/readiness/breakdown/evidence) so all fixture shape changes
are made in one place.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro

Run ID: c88acada-9a53-4c76-b74d-4b388cd7e91a

📥 Commits

Reviewing files that changed from the base of the PR and between ab88bfb and 1aa2b74.

📒 Files selected for processing (9)
  • .github/workflows/cert-shipme.yml
  • .github/workflows/wesley-holmes.yml
  • CHANGELOG.md
  • CHRONICLES_OF_THE_MACHINE-KIND_VOL_00000010.jsonl
  • packages/wesley-cli/src/commands/cert-create.mjs
  • packages/wesley-cli/src/commands/cert-verify.mjs
  • packages/wesley-cli/test/cert-e2e.bats
  • schemas/shipme.schema.json
  • test/ci-workflows.bats

Comment thread packages/wesley-cli/test/cert-e2e.bats Outdated
Comment thread packages/wesley-cli/test/cert-e2e.bats Outdated
@flyingrobots

Copy link
Copy Markdown
Owner Author

@coderabbitai review please 🥕

@github-actions

github-actions Bot commented Mar 25, 2026 •

Copy link
Copy Markdown

🚢 SHIPME

[SHIPME] PASS · HOLMES ELEMENTARY — sha 6336ced

@coderabbitai

coderabbitai Bot commented Mar 25, 2026

Copy link
Copy Markdown

@flyingrobots: On it! Kicking off a fresh review of PR #463 now. 🐇🥕


🧠 Learnings used
Learnt from: flyingrobots
Repo: flyingrobots/wesley PR: 0
File: :0-0
Timestamp: 2026-03-25T13:51:01.105Z
Learning: In `flyingrobots/wesley`, `.github/workflows/cert-shipme.yml` requires `pull-requests: write` (not just `issues: write`) for the "Create/Update PR Comment" step to succeed. Run `23530391194` confirmed that `issues: write` alone caused "Resource not accessible by integration" even though the script uses the GitHub Issues REST API. This is a known GitHub Actions quirk where certain trigger shapes gate PR comment creation on `pull-requests: write` at the integration layer. The regression guard in `test/ci-workflows.bats` intentionally asserts this permission is present. Do not flag this as an unnecessary permission widening without first checking this empirical context.

Learnt from: CR
Repo: flyingrobots/wesley PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-02-12T15:12:26.547Z
Learning: Applies to .github/workflows/**/*.{yml,yaml} : Use least privilege in workflows; do not widen permissions or add secrets without approval

Learnt from: CR
Repo: flyingrobots/wesley PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-02-12T15:12:26.547Z
Learning: Applies to .github/workflows/**/*.{yml,yaml} : Only touch GitHub Actions workflows when fixing a deterministic break or reducing obvious runtime/duplication, and provide a revert path in the PR description

Learnt from: flyingrobots
Repo: flyingrobots/wesley PR: 400
File: packages/wesley-cli/src/framework/irToSchema.mjs:0-0
Timestamp: 2026-03-08T13:56:43.735Z
Learning: In `flyingrobots/wesley`, `packages/wesley-cli/src/framework/irToSchema.mjs` was rewritten in commit 042fd61 (PR `#400`). The rewritten adapter now expects the IR to use `t.fields[]` (not `t.columns[]`) with structured `FieldType` objects (`f.type.base`, `f.type.isList`, `f.nullable`) rather than plain PostgreSQL type strings. Table directives are passed through via `t.directives || {}`. Field directives are translated from structured IR flags (`pk`, `fk`, `unique`, `default`, `index`) to domain `@`-prefixed format by `buildFieldDirectives`. The prior learning about `t.columns` with plain PG strings (canonical runtime IR) is now superseded for this adapter file.

Learnt from: flyingrobots
Repo: flyingrobots/wesley PR: 463
File: CHRONICLES_OF_THE_MACHINE-KIND_VOL_00000010.jsonl:75-75
Timestamp: 2026-03-25T07:48:34.953Z
Learning: In flyingrobots/wesley, Chronicle entries in CHRONICLES_OF_THE_MACHINE-KIND_VOL_*.jsonl may contain fractional-second UTC timestamps (e.g., 2026-03-25T06:08:24.227Z). Multiple such entries are already committed to VOL_00000010. The append-only policy prohibits in-place mutation of historical entries, so timestamp precision must be enforced via the append tooling going forward — not by suggesting in-place edits during review. Flagging individual committed entries for fractional seconds is policy-violating noise; enforcement belongs in the tooling layer.

Learnt from: flyingrobots
Repo: flyingrobots/wesley PR: 400
File: packages/wesley-cli/src/framework/irToSchema.mjs:0-0
Timestamp: 2026-03-08T13:56:14.415Z
Learning: In the `flyingrobots/wesley` repository, `packages/wesley-cli/src/framework/irToSchema.mjs` (post-commit 042fd61) reads the CURRENT RUNTIME IR which uses structured `FieldType` objects. The IR now has `ir.tables[].fields[]` (NOT `columns[]`), where each field has `f.type.base` (a GraphQL scalar name like `"ID"`, `"String"`, `"DateTime"`, `"JSON"`), `f.type.isList` (boolean), and `f.nullable` (boolean). There is NO PostgreSQL-to-GraphQL mapping (`PG_TO_GQL`) in this file anymore. The adapter is a straightforward pass-through from the structured IR to core `Schema`/`Table`/`Field` objects. The `WesleyIR.schema.ts` shape with `{ base, isList, listItemNullable }` IS the current live runtime IR shape as of this migration.

Learnt from: CR
Repo: flyingrobots/wesley PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-02-12T19:09:57.726Z
Learning: Applies to .github/workflows/cli-*.{yml,yaml} : Dedicated CLI workflows (e.g., cli-tests.yml, cli-quick.yml) own CLI end-to-end/bats checks; avoid duplicating them in main CI without approval
✅ Actions performed

Review triggered.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 4

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
.github/workflows/wesley-holmes.yml (1)

742-750: ⚠️ Potential issue | 🟡 Minor

Duplicated "Ensure history for MORIARTY" step in the wrong job.

Lines 742–750 are an exact duplicate of lines 222–229. This step belongs in the moriarty-predict job (where it already exists), not in comment-report. The comment-report job doesn't run any MORIARTY commands—it only assembles and posts a PR comment from downloaded artifacts.

Delete this orphaned step; it serves no purpose and clutters the workflow.

🗑️ Proposed removal
             if (botComment) {
               await github.rest.issues.updateComment({
                 owner: context.repo.owner,
                 repo: context.repo.repo,
                 comment_id: botComment.id,
                 body
               });
             } else {
               await github.rest.issues.createComment({
                 owner: context.repo.owner,
                 repo: context.repo.repo,
                 issue_number: context.issue.number,
                 body
               });
             }
-      - name: "🔄 Ensure history for MORIARTY"
-        shell: bash
-        run: |
-          set -euo pipefail
-          # In case fetch-depth wasn't honored or repo is shallow, try to unshallow
-          if git rev-parse --is-shallow-repository >/dev/null 2>&1; then
-            git fetch --prune --unshallow --tags || true
-          fi
-          git fetch --prune origin '+refs/heads/*:refs/remotes/origin/*' || true
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In @.github/workflows/wesley-holmes.yml around lines 742 - 750, The workflow
contains a duplicated step named "🔄 Ensure history for MORIARTY" that is
incorrectly present in the comment-report job; remove that orphaned step from
the comment-report job so the only instance remains in the moriarty-predict job
(keep the existing step in moriarty-predict, delete the block with name "🔄
Ensure history for MORIARTY" under the comment-report job).
♻️ Duplicate comments (2)
packages/wesley-cli/test/cert-e2e.bats (2)

125-286: 🧹 Nitpick | 🔵 Trivial

Fixture helpers are duplicated—consider consolidating to reduce drift risk.

create_holmes_summary_inputs() (lines 125–222) and create_passing_holmes_summary_inputs() (lines 224–286) share ~90% identical JSON structure. When the HOLMES bundle schema evolves, you'll need to update both in lockstep or risk silent test rot.

Extract a parameterized helper that accepts score/coverage values and emits the fixture. This is a maintenance hygiene concern, not a blocker.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@packages/wesley-cli/test/cert-e2e.bats` around lines 125 - 286, Both
create_holmes_summary_inputs() and create_passing_holmes_summary_inputs()
duplicate the same fixture creation logic; refactor by extracting a single
parameterized helper (e.g., generate_holmes_summary_fixture) that accepts the
varying values (scores.tci, scores.scs.tests, tci.e2e_ops.covered/total,
evidence.tests lines, etc.) and is invoked by the two existing functions to
write schema.sql, tests.sql, .wesley-cache/bundle.json and
.wesley-cache/scores.json; update create_holmes_summary_inputs and
create_passing_holmes_summary_inputs to call that helper with the appropriate
parameters so the shared JSON structure lives in one place.

136-187: 🧹 Nitpick | 🔵 Trivial

Fixtures omit testResults, masking Holmes verification metric paths.

Neither create_holmes_summary_inputs nor create_passing_holmes_summary_inputs includes a testResults block in the bundle. Holmes.investigationData() derives verification/execution/conclusion metrics from bundle.testResults. Without it, the E2E tests don't exercise those code paths, which could mask regressions in policy-facing summary behavior.

Consider adding a minimal testResults to at least one fixture to exercise the full investigation flow.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@packages/wesley-cli/test/cert-e2e.bats` around lines 136 - 187, The fixture
bundle JSON is missing a testResults block so Holmes.investigationData() never
exercises verification/execution/conclusion metrics; add a minimal testResults
object to the fixture produced by create_holmes_summary_inputs (or
create_passing_holmes_summary_inputs) that mirrors the shape Holmes expects
(e.g., overall verdict/status, at least one test entry and timing/execution
fields) so bundle.testResults is present and the full investigation flow runs,
ensuring Holmes.investigationData() can compute verification metrics.
🤖 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/workflows/wesley-holmes.yml:
- Around line 141-142: The workflow validates and accepts the input schema-path
but never forwards it to the CLI, so update the run step that invokes cli.mjs
(the call that currently passes --bundle-dir, --history-file, --json) to also
pass --schema-path with the value from the action input (schema-path), or
alternatively remove schema-path from the action inputs and callers if the CLI
does not require it; ensure changes reference the run-holmes-command action
input named schema-path and the cli.mjs invocation so the validated path is
actually used by the tool.

In `@packages/wesley-core/src/application/Scoring.mjs`:
- Around line 392-393: The calculateIndexCoverage method currently returns 1
when total === 0, inconsistent with its sibling methods
(calculateTestCoverageDetails, calculateConstraintCoverageDetails,
calculateRelationCoverageDetails, calculateRlsCoverageDetails) which return 0
for empty sets; change calculateIndexCoverage to return total > 0 ? covered /
total : 0 to match the others, and add a short comment above the return
explaining that an empty set yields 0 coverage for consistency with other
coverage calculations (or alternatively update the other methods if you prefer
the vacuous-truth semantics—be consistent across all calculate*Coverage
methods).

In `@packages/wesley-core/test/unit/scoring-engine.test.mjs`:
- Around line 176-177: Add a regression unit test that ensures
calculateIndexCoverage on ScoringEngine returns 0 when there are indexed fields
but no test evidence recorded: create an EvidenceMap (do not add index evidence
but set a sha), build a schema/table with fields including an indexed field (use
createField for 'email' with { indexed: true } and a primary key field), provide
a schema object with getTables -> [table] and table.getFields -> fields,
instantiate new ScoringEngine(evidence) and assert
scoring.calculateIndexCoverage(schema, []) === 0 to lock in the total>0 branch
behavior.
- Line 148: Rename the ambiguous test title to explicitly reflect scenario A (no
indexed fields present): update the test invocation test('TCI treats missing
index obligations as fully covered', ...) to a clearer name like test('TCI
treats absent index obligations (schema has no indexed fields) as fully
covered', ...) so the test for no-index-schema behavior is unambiguous; leave
the test body and assertions unchanged.

---

Outside diff comments:
In @.github/workflows/wesley-holmes.yml:
- Around line 742-750: The workflow contains a duplicated step named "🔄 Ensure
history for MORIARTY" that is incorrectly present in the comment-report job;
remove that orphaned step from the comment-report job so the only instance
remains in the moriarty-predict job (keep the existing step in moriarty-predict,
delete the block with name "🔄 Ensure history for MORIARTY" under the
comment-report job).

---

Duplicate comments:
In `@packages/wesley-cli/test/cert-e2e.bats`:
- Around line 125-286: Both create_holmes_summary_inputs() and
create_passing_holmes_summary_inputs() duplicate the same fixture creation
logic; refactor by extracting a single parameterized helper (e.g.,
generate_holmes_summary_fixture) that accepts the varying values (scores.tci,
scores.scs.tests, tci.e2e_ops.covered/total, evidence.tests lines, etc.) and is
invoked by the two existing functions to write schema.sql, tests.sql,
.wesley-cache/bundle.json and .wesley-cache/scores.json; update
create_holmes_summary_inputs and create_passing_holmes_summary_inputs to call
that helper with the appropriate parameters so the shared JSON structure lives
in one place.
- Around line 136-187: The fixture bundle JSON is missing a testResults block so
Holmes.investigationData() never exercises verification/execution/conclusion
metrics; add a minimal testResults object to the fixture produced by
create_holmes_summary_inputs (or create_passing_holmes_summary_inputs) that
mirrors the shape Holmes expects (e.g., overall verdict/status, at least one
test entry and timing/execution fields) so bundle.testResults is present and the
full investigation flow runs, ensuring Holmes.investigationData() can compute
verification metrics.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro

Run ID: 8d51fc6d-337b-4d3c-9243-df98f0edf75a

📥 Commits

Reviewing files that changed from the base of the PR and between ab88bfb and f4e3c6b.

📒 Files selected for processing (11)
  • .github/workflows/cert-shipme.yml
  • .github/workflows/wesley-holmes.yml
  • CHANGELOG.md
  • CHRONICLES_OF_THE_MACHINE-KIND_VOL_00000010.jsonl
  • packages/wesley-cli/src/commands/cert-create.mjs
  • packages/wesley-cli/src/commands/cert-verify.mjs
  • packages/wesley-cli/test/cert-e2e.bats
  • packages/wesley-core/src/application/Scoring.mjs
  • packages/wesley-core/test/unit/scoring-engine.test.mjs
  • schemas/shipme.schema.json
  • test/ci-workflows.bats

Comment thread .github/workflows/wesley-holmes.yml
Comment thread packages/wesley-core/src/application/Scoring.mjs Outdated
Comment thread packages/wesley-core/test/unit/scoring-engine.test.mjs Outdated
Comment thread packages/wesley-core/test/unit/scoring-engine.test.mjs

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@packages/wesley-cli/test/cert-e2e.bats`:
- Around line 453-458: Replace the combined jq patterns that use
'select(has("X")) | .X == Y' with two explicit checks so missing-field vs
bad-value are distinguishable: first run jq -e 'has("holmesPassed")' and then jq
-e '.holmesPassed == false', do the same for 'holmesVerdict' (check
has("holmesVerdict") then '.holmesVerdict == "REQUIRES INVESTIGATION"') and for
'eligibleToShip' (has("eligibleToShip") then '.eligibleToShip == false'); update
the three echo "$output" | jq -e ... lines to perform these two-step assertions
for fields holmesPassed, holmesVerdict, and eligibleToShip.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro

Run ID: aa1465c5-6450-478f-bb0a-92836a34a93b

📥 Commits

Reviewing files that changed from the base of the PR and between f4e3c6b and 82ef427.

📒 Files selected for processing (5)
  • CHANGELOG.md
  • CHRONICLES_OF_THE_MACHINE-KIND_VOL_00000010.jsonl
  • packages/wesley-cli/test/cert-e2e.bats
  • packages/wesley-core/src/application/Scoring.mjs
  • packages/wesley-core/test/unit/scoring-engine.test.mjs

Comment thread packages/wesley-cli/test/cert-e2e.bats

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 2

🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@CHANGELOG.md`:
- Around line 69-74: Remove the PR-thread meta phrase "This keeps the reviewer’s
'field exists vs wrong value' distinction while staying robust when the command
also emits the framework error envelope" and rewrite that sentence to state the
observable effect: that the HOLMES failure-path cert E2E tests now assert
against the first JSON document emitted by `cert-verify --json`, separating
presence checks from value checks for `holmesPassed`, `holmesVerdict`,
`eligibleToShip`, and `reasons`, so the tests remain robust when the command
also emits an outer error envelope (PR `#463`).

In `@packages/wesley-cli/test/cert-e2e.bats`:
- Around line 149-246: The test fixture currently duplicates the same scoring
payload in the .wesley-cache/bundle.json and .wesley-cache/scores.json files;
consolidate by emitting the scores object once and reusing it in both outputs:
generate the scores JSON into a single variable/file (e.g., a SCORE_PAYLOAD
variable or a temporary .wesley-cache/scores.json) and then embed or include
that single source when creating bundle.json (replace the duplicated "scores": {
... } block in the bundle creation with the shared payload reference), ensuring
the same canonical payload is written to both the scores file and the bundle's
top-level scores.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro

Run ID: a2c748ed-110c-45df-a791-e6bb04f7ef2d

📥 Commits

Reviewing files that changed from the base of the PR and between f4e3c6b and 7b0f3f5.

📒 Files selected for processing (5)
  • CHANGELOG.md
  • CHRONICLES_OF_THE_MACHINE-KIND_VOL_00000010.jsonl
  • packages/wesley-cli/test/cert-e2e.bats
  • packages/wesley-core/src/application/Scoring.mjs
  • packages/wesley-core/test/unit/scoring-engine.test.mjs

Comment thread CHANGELOG.md Outdated
Comment thread packages/wesley-cli/test/cert-e2e.bats Outdated
@flyingrobots
flyingrobots merged commit 0b99848 into main Mar 30, 2026
30 checks passed
@flyingrobots
flyingrobots deleted the feature/active-next-slice branch March 30, 2026 07:20
@coderabbitai coderabbitai Bot mentioned this pull request Jun 27, 2026
12 tasks done
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request feature New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

feat(ops): infer search_path from IR schema references

1 participant