Skip to content

Fold finmod feedback (rounds 1-2) into the verbatim fleet-law fixes - #409

Merged
ptr727 merged 5 commits into
developfrom
verifdiscipline-generic-and-audit-clone-note
Jul 23, 2026
Merged

Fold finmod feedback (rounds 1-2) into the verbatim fleet-law fixes#409
ptr727 merged 5 commits into
developfrom
verifdiscipline-generic-and-audit-clone-note

Conversation

@ptr727

@ptr727ptr727 commented Jul 23, 2026

Copy link
Copy Markdown
Owner

Addresses the finmod agent's feedback (rounds 1 and 2) on vendored fleet-law that downstream cannot fix without breaking the verbatim hash.

Round 1

  • Self-contain the Verification Discipline section. Generalized two cross-refs that pointed at hub-only WORKFLOW.md anchors, so the verbatim section carries clean into a source-only repo.
  • Document the audit's full-clone need. AUDIT.md now states that stale-vs-modified discrimination needs the canonical's git history, with the verify-against-current-main fallback for a shallow verifier.

Round 2

  • Scope the write-safety rule to what it protects - hiding a failure. An ad-hoc call may never suppress its error stream or force success; a reviewed set -e script may drop a write's stdout only, because stderr stays visible and a failed write still aborts loudly. This makes the rule and the reference repo-config/configure.sh agree (and backs Apply PhotoCleaner round-2 feedback: rationale wording, MD033 clarity, CPM property owner #408's rationale comment rather than obsoleting it).
  • Kill the two clause-joining semicolons in the Verification Discipline "flags an instance - fix the class" bullet, which violated the section's own no-clause-semicolon rule.
  • Guard the two unguarded gh api reads in configure.sh check mode, so a transient read records a FAIL and continues instead of aborting the auditor.
  • Warn at the per_page=100 cap in ruleset_id instead of silently narrowing the lookup.

Touches two verbatim sections (write-safety + Verification Discipline); the fleet re-vendors both on promotion, tracked by the burn-down report.

…'s full-clone need
From the finmod round-1 convergence feedback (Financial-Modeling#48). Both items are structural
tensions the audit does not catch, surfaced by re-vendoring into a source-only repo.
- The verbatim "Verification Discipline" section carried two parentheticals that cite hub
structure a source-only repo lacks - WORKFLOW.md D8.4 (no D-numbered sections there) and the
changes-job under Branching Model (no smoke/changes job in a source-only Python repo) - so a
byte-identical carry dangled those refs. A verbatim section must be self-contained: the
fail-closed rule now states the identity-allowlist example inline without the WORKFLOW.md
cross-link, and the confirm-from-the-log rule drops the changes-job pointer. The rules are
unchanged; only the repo-specific cross-refs are gone. The fleet re-vendors this section
(its hash changed) - tracked by the burn-down.
- AUDIT.md section 8 now documents that re-running the audit needs a full hub clone with git
history (the stale-vs-modified classification walks git log/show), and gives the
verify-one-finding-against-main fallback for a downstream agent without the history.
Not changed: the finmod-local cross-ref from its "Pull Request Title and Commit Message
Conventions" intent section to a co-author-trailer rule that the verbatim Git and Commit Rules
does not carry. Per the Financial-Modeling#28/#379 disposition the trailer stays a repo-local
convention ("no Co-Authored-By unless asked"; self-credit is the standing-asked case), so the
downstream repo self-contains that note rather than the hub codifying it.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
CopilotAI review requested due to automatic review settings July 23, 2026 18:03

CopilotAI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

This PR updates the hub governance documentation to make two “Verification Discipline” references portable across repo types and to document a practical constraint of re-running the audit from non-full checkouts.

Changes:

  • Removes repo-structure-dependent cross-references inside the “Verification Discipline” section so the text remains valid when vendored verbatim into source-only repos.
  • Documents that re-running spec/audit.py’s stale-vs-modified verbatim classification requires a full clone of this repo with git history, and suggests a history-free fallback comparison approach.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

FileDescription
AGENTS.mdGeneralizes two “Verification Discipline” bullets by removing cross-references that don’t exist in all repo shapes.
AUDIT.mdAdds documentation about the audit’s dependency on local git history for verbatim stale-vs-modified classification and a fallback approach for downstream agents.

Comment threadAUDIT.md Outdated
…thout a fragile command
Drop the inline gh api example - it hard-coded the hub slug (wrong to reproduce verbatim in a
carried doc) and showed only the whole-file case. The fallback now reads generically: compare
against the current hub canonical on main, the whole file for a file-level unit or the named
heading block for a verbatim section.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
CopilotAI review requested due to automatic review settings July 23, 2026 18:06

CopilotAI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

Comment threadAGENTS.md Outdated
The bullet is about failing loud (a silent gate produces an error/annotation), so the example
now says the allowlist must raise an error when its list stops matching rather than silently
passing everything - "fail closed" was the wrong concept for a visibility rule.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
CopilotAI review requested due to automatic review settings July 23, 2026 18:09

CopilotAI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated no new comments.

Four items the finmod agent surfaced during its convergence PR, all on
byte-identical vendored content that downstream cannot fix without breaking
the verbatim hash:
- Scope the write-safety "never suppress a write's output" rule to what it
protects - hiding a failure. An ad-hoc call may never suppress its error
stream or force success; a reviewed set -e script may drop a write's stdout
only, because stderr stays visible and a failed write still aborts loudly.
This makes the rule and the reference repo-config/configure.sh agree.
- Recast the two clause-joining semicolons in the Verification Discipline
"flags an instance - fix the class" bullet, which violated the section's
own no-clause-semicolon style rule.
- Guard the two unguarded gh api reads in configure.sh check mode so a
transient read records a FAIL and continues instead of aborting the auditor.
- Warn when ruleset_id hits the per_page cap instead of silently narrowing
the lookup.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
CopilotAI review requested due to automatic review settings July 23, 2026 19:37

CopilotAI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.

Comment threadrepo-config/configure.sh Outdated
A truncation warning that continues could let apply mode miss a ruleset past
the first 100 and create a duplicate by name. Abort instead - the caller
already treats a non-zero return as stop, not not-found.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
CopilotAI review requested due to automatic review settings July 23, 2026 19:41

CopilotAI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 3 out of 3 changed files in this pull request and generated no new comments.

@ptr727ptr727 changed the title Generalize two Verification Discipline cross-refs; document the audit's full-clone needFold finmod feedback (rounds 1-2) into the verbatim fleet-law fixesJul 23, 2026
@ptr727
ptr727 merged commit 6dc5ecb into developJul 23, 2026
7 checks passed
ptr727 added a commit that referenced this pull request Jul 23, 2026
Follow-up to #409 from the promotion review: check_ruleset invoked ruleset_id unguarded, so its non-zero return would exit the auditor via set -e instead of recording a FAIL. Wrap it in if/fail/return, consistent with the sibling gh-read guards, and document the per_page-cap abort in the header.
ptr727 added a commit that referenced this pull request Jul 23, 2026
…fixes (#410)
Carries #408 (PhotoCleaner round-2), #409 (finmod rounds 1-2 write-safety carve-out + configure.sh guards), #411 (check-mode ruleset_id guard), #412 (DSC verbatim semicolon sweep). Merge commit, no release.
@ptr727
ptr727 deleted the verifdiscipline-generic-and-audit-clone-note branch July 23, 2026 21:36
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants

@ptr727