Skip to content

feat(intelligence): accept directly authored profiles - #659

Open
drewstone wants to merge 1 commit into
mainfrom
feat/direct-profile-candidate
Open

feat(intelligence): accept directly authored profiles#659
drewstone wants to merge 1 commit into
mainfrom
feat/direct-profile-candidate

Conversation

@drewstone

Copy link
Copy Markdown
Contributor

Summary

  • derive the existing AgentProfileDiff sequence from any exact baseline/candidate AgentProfile pair
  • activate directly authored profiles through the existing measured experiment, review, and activation lifecycle
  • recompute profile comparisons from their signed receipts before proposal or activation
  • remove the granular shortcut that could re-add unchanged external resources

No new candidate type or parallel lifecycle is introduced.

Proof

  • 570/570 test files passed: 1,862 passed, 6 skipped, 0 failed
  • typecheck passed, including examples
  • lint passed across 502 files
  • build and docs:check passed
  • exact profile transition matrix: 48/48
  • current origin/main merge-tree succeeded

@tangletools tangletools 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.

✅ Auto-approved drewstone PR — 1d1badab

This PR was opened by the trusted drewstone account.
The full PR reviewer audit still runs separately and will publish findings if it detects issues.

tangletools · auto-approval · reason: drewstone_author · 2026-07-29T21:28:13Z

@tangletools tangletools 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.

🟢 Value Audit — sound

Verdict sound
Concerns 0 (none)
Heuristic 0.0s
Duplication 0.0s
Interrogation 276.7s (2 bridge agents)
Total 276.7s

💰 Value — sound

Extends the measured profile-improvement path to accept completely authored AgentProfiles by deriving exact field-level diffs — coherent, in-grain, no existing capability duplicated.

  • What it does: Before this change, the measured profile-improvement path could only express 'prompt' and 'skills' surface changes (AGENT_PROFILE_MEASURED_SURFACES = ['prompt','skills']); any other profile field (model, permissions, tools, hooks, mcp, subagents, identity, metadata, etc.) was rejected with 'measured profile contract cannot apply'. Now agentImprovementProfileDiffs computes a whole-profile reset+set
  • Goals it achieves: 1) Let product-owned directly-authored profiles use the same measured lifecycle as optimizer-produced granular changes — no parallel candidate type or lifecycle. 2) Fix a real correctness gap: the old per-surface shortcut could re-add unchanged external resources (GitHub-hosted skills/tools) because AgentProfileDiff.set uses array-append semantics (mergeAgentProfiles); the new field-level differ o
  • Assessment: Coherent and well-executed. The central design decision — emit a reset diff (remove only changed fields) followed by a set diff (set only changed fields), rather than a blunt whole-profile overwrite — is forced by the append semantics of applyAgentProfileDiff (confirmed in agent-interface/dist/profile-diff.d.ts:39-40: 'set is an AgentProfile overlay: profile arrays are appended'). A blunt overwrit
  • Better / existing approach: none — this is the right approach. Searched for an existing 'compute diff between two profiles' primitive: agent-interface/dist/profile-diff.d.ts exports only applyAgentProfileDiff (apply), changedAgentProfileAxes (axes changed BY a diff), pruneAgentProfileDiff (prune), and defineAgentProfileDiff (identity) — none compute a diff FROM two profiles, so the field-level differ is new and necessary. Th
  • Model: opencode/zai-coding-plan/glm-5.2
  • Bridge attempts: 2
  • Bridge warning: opencode/kimi-for-coding/k2p7: bridge stream ended without value-audit content

🎯 Usefulness — sound

A coherent extension of the existing measured profile-improvement lifecycle to directly-authored complete profiles, with a correct fix for unchanged-resource leakage and stronger receipt-based verification; no parallel lifecycle introduced and no dead surface.

  • Integration: Fully reachable. The new complete-profile differ (agentImprovementProfileDiffs -> completeAgentProfileReplacementDiffs) is invoked by the public proposeAgentProfileImprovement (improvement-cycle.ts:824); the resulting agent-profile surface is admitted by the propose guard (improvement-cycle.ts:765) and by profileTargets in the activation path (profile-activation.ts:358, via isAgentProfileMeasuredS
  • Fit with existing patterns: Fits the codebase grain precisely. It deliberately avoids a new candidate type or parallel lifecycle (confirmed by code): it widens the existing AgentProfileMeasuredSurface union with 'agent-profile', reuses applyExactAgentProfileDiff as the applier, the existing experiment sealing, and the profile-transition activation mechanism. The switch from agentProfileImprovementMeasuredComparisonSchema.par
  • Real-world viability: Robust beyond the happy path. The differ round-trip-verifies (canonicalCandidateDigest(applied) !== candidate -> throw, improvement-surfaces.ts:408-410) so any non-reproducing diff is rejected before it ships. profileValuesDiffer normalizes absent-vs-present correctly; resource sub-fields are only included when they actually change (replaceChangedProfileResources), which is the fix for the prior b
  • Model: opencode/zai-coding-plan/glm-5.2
  • Bridge attempts: 1

No concerns — sound change, no better or existing approach found. ✅


What this audit checks

It judges the change on its merits — not whether it was tasked out in an issue. Unticketed, fast-moving work is fine; the question is whether the change is good and whether a better or existing approach should be used instead.

Pass What it asks
Heuristic Vague title? Whitespace-only or cruft-bearing diff? (content signals only)
Duplication Do added function/class names already exist elsewhere in the repo?
Value Audit What does it do? What goal does it achieve? Is it good? Better architecture or already-exists?
Usefulness Audit Does it integrate and fit? Will it hold up in real use and actually get used?

Findings are concerns, not blocks — the human reviewer decides what to do with them.

value-audit · 20260729T213305Z

@tangletools

Copy link
Copy Markdown
Contributor

❌ Needs Work — 1d1badab

Review health 100/100 · Reviewer score 0/100 · Confidence 95/100 · 9 findings (4 critical, 1 high, 4 low)

glm: Correctness 0 · Security 0 · Testing 0 · Architecture 0

Reviewer score is advisory once the run is complete and the verdict has no blockers.

Full multi-shot audit completed 7/7 planned shots over 10 changed files. Global verifier still owns final merge decision.

Blocking

🟣 CRITICAL Profile improvement schema rejects non-prompt/skills changes; 4 new tests fail — src/intelligence/improvement-surfaces.ts

profileImprovementChangedSurfaces delegates to changedProfileImprovementSurfaces from @tangle-network/agent-interface@0.36.0. That function (verified in node_modules/.pnpm/@tangle-network+agent-interface@0.36.0/.../agent-profile-improvement-schema.js) only inspects step.set?.prompt/step.remove?.prompt and step.set?.resources?.skills/step.remove?.resources?.skills. The new completeAgentProfileReplacementDiffs emits diffs over model, harness, tools, mcp, connections, subagents, hooks, modes, confidential, metadata, extensions, etc. with metadata.surface='agent-profile' (improvement-surfaces.ts:491-498). The same changedProfileImprovementSurfaces is used by agentProfileImprovementChangeSchema.superRefine in agent-interface, so `sealAgentProfileImpro

🟣 CRITICAL New 'directly authored complete profile' test fails: pinned contract only allows prompt/skills surfaces — tests/improvement-cycle.test.ts

Executed npx vitest run tests/improvement-cycle.test.ts -t 'measures and activates a directly authored complete profile': ZodError thrown at sealAgentProfileImprovementExperiment({...}) (tests/improvement-cycle.test.ts:361). The candidateProfile changes model/harness/tools/mcp/hooks/metadata, so agentImprovementProfileDiffs (src/intelligence/improvement-surfaces.ts:386, modified in this PR) emits diffs with set.model/set.harness/set.tools/set.mcp/set.hooks/set.metadata and remove touching those keys. But @tangle-network/agent-interface@0.36.0's changedProfileImprovementSurfaces + the experiment sealer (agent-eval@0.135.2 profile-measured-comparison.ts:117) only tolerate prompt and skills: they emit 'profile improvement patch must change prompt or skills', 'profile improvement patches m

🟣 CRITICAL 3 tests fail: expanded surface lists unsupported by any published agent-interface version — tests/improvement-surfaces.test.ts

Lines 449-457 assert profileImprovementChangedSurfaces(completeChanges) equals ['prompt','skills','tools','mcp','hooks','subagents','agent-profile']. Actual result: ['prompt','skills']. The function delegates to changedProfileImprovementSurfaces from @tangle-network/agent-interface@0.36.0, whose source (verified in node_modules and npm 0.36.0–0.38.0) only inspects step.set?.prompt/step.remove?.prompt and step.set?.resources?.skills/step.remove?.resources.skills. Same failure at line 572 (removal-on

🟣 CRITICAL Model-only (and any non-prompt/non-skills) profile change crashes surface reporting — tests/improvement-surfaces.test.ts

Line 491 calls profileImprovementChangedSurfaces(changes) for a model-only change (baseline model differs from candidate model, no prompt/skills delta). The upstream changedProfileImprovementSurfaces returns [] (model is not prompt or skills), so the local wrapper at improvement-surfaces.ts:207 throws 'profile improvement experiment does not change a supported surface'. The earlier toMatchObject and round-trip .toEqual(candidate) assertions at lines 480-490 PASS, proving the diff is correctly produced —

🔴 HIGH changedSurfaces silently underreports for mixed prompt/skills + other-axis changes — src/intelligence/improvement-surfaces.ts

Even when the schema accepts a change (because prompt or skills is among the changed axes), profileImprovementChangedSurfaces returns only the recognized subset. validateShippableAgentImprovementEvaluation (improvement-cycle.ts:1194-1197) stores this subset as the proposal's changedSurfaces. A change that alters prompt + tools + mcp + hooks + subagents would be reported as just ['prompt']. Effect: (1) audit/review metadata understates the change scope; (2) assertAgentImprovementActivationTargets (improvement-surfaces.ts:134-161) compares activation target surfaces against this subset via sameAgentImprovementSurfaceSet, so an activation that names {tools, mcp, hooks, agent-profile} against a proposal advertising only {prompt} throws 'must cover exactly the changed surfaces'.

Other

🟡 LOW Removal-only diff for non-prompt/skills axes has no integration coverage that actually runs — src/intelligence/improvement-surfaces.ts

The 'reproduces a complete profile using removal-only changes' test (improvement-surfaces.test.ts:567-583) exercises the Object.keys(set).length === 0 branch at line 507 that returns only a reset diff, but the test never reaches the surface-mapping assertion because it throws earlier in profileImprovementChangedSurfaces. The branch is therefore unverified end-to-end. Once the dep gap is fixed, this test will cover it; until then it is dead coverage.

🟡 LOW Degenerate confidence interval reduces the fixture's discriminating power as a regression sentinel — src/testing/fixtures/agent-profile-improvement-proposal.json

The regenerated fixture uses synthetic receipts with baseline=0 and candidate=1 for all 6 reps, so the production paired-bootstrap correctly returns a zero-width CI [1,1] and minimumDetectableDelta=0 (evaluation.power, line 1818). The prior fixture hand-rolled a non-degenerate CI [0.3,0.5] that was statistically inconsistent with its own zero-variance inputs. The new version is more honest, but as a regression sentinel it no longer exercises the 'CI straddles the decision boundary' path. Not a blocker — the fixture's job is to validate the proposal envelope and digests, which it does — but if the team later wants the fixture to

🟡 LOW Conditional spread for optional metadata is needlessly verbose — tests/helpers/profile-improvement-fixture.ts

The expression ...(options.metadata ? { metadata: options.metadata } : {}) guards an already-optional field. Since metadata? is optional on CompareAgentProfileImprovementExperimentOptions (agent-eval contract index.d.ts:546), this can be the plain metadata: options.metadata. Pure style; no behavioral difference. Fix only if touching the file again.

🟡 LOW Test title rename drops coverage of the previous rejection contract — tests/improvement-surfaces.test.ts

The old test ('derives measured prompt and skill changes and rejects other profile changes') asserted that unsupported profile changes (tools/mcp/hooks/subagents on candidate) threw /measured profile contract cannot apply/. The PR replaces this with the new complete-profile acceptance path, which is intentional (the feature now supports those surfaces), but no test covers the case where an unsupported field still exists on a profile that the differ cannot represent. Verify with the PR author that every AgentProfile field is now handled — the source has compile-time exhaustiveness guards (improvement-surfaces.ts:84-86), so this is likely fine, but the behavioral guard test was removed.


tangletools · 2026-07-29T21:46:12Z · trace

@tangletools tangletools 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.

❌ 5 Blocking Findings — 1d1badab

Full multi-shot audit completed 7/7 planned shots over 10 changed files. Global verifier still owns final merge decision.

Full immutable report for this review: trace

Summary comment for this run: full summary


tangletools · 2026-07-29T21:46:12Z · immutable trace

Sign up for free to 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