---
name: fp-go-pr-review
description: >-
  Use this skill when reviewing pull requests for fp-go code
  (github.com/IBM/fp-go/v2). Trigger on mentions of PR review, code review,
  pull request validation, fp-go best practices validation, functional
  programming review, or when the user asks to review changes on a PR branch.
  This skill validates that changes follow fp-go conventions including
  data-last composition, point-free style, proper monad usage, lens patterns,
  and idiomatic functional patterns.
---

# fp-go PR Review

## Overview

This skill assists with reviewing pull requests that use the fp-go library (github.com/IBM/fp-go/v2). It validates that code changes follow fp-go best practices and functional programming conventions. Requires Go 1.24+ for generic type alias support.

## When to Use This Skill

- Reviewing pull requests with fp-go code
- Validating that changes follow fp-go best practices
- Checking for common fp-go anti-patterns
- Ensuring proper functional composition patterns
- Verifying correct monad usage and error handling

## Review Checklist

### 1. Import Path Validation

**Rule**: All imports MUST use `github.com/IBM/fp-go/v2/...`, never `github.com/IBM/fp-go/...` (v1).

**Check for**:
```go
// ❌ WRONG - v1 import
import "github.com/IBM/fp-go/option"

// ✅ CORRECT - v2 import
import O "github.com/IBM/fp-go/v2/option"
```

**Severity**: Critical — v1 and v2 are incompatible

Also check that import aliases follow the canonical table in the **fp-go** skill
(`R` = `result`, `RD` = `reader`, `P` = `predicate`, `PA` = `pair`, `EM` = `endomorphism`,
`IOR` = `ioresult`, `L` = `optics/lens`, `logging` unaliased, …). The same letter meaning
two packages across files is a Low finding.

### 2. Data-Last Principle

**Rule**: All fp-go operations use data-last. The data being transformed is always the last argument.

**Check for**:
```go
// ❌ WRONG - data-first
option.Map(myOption, transformFunc)

// ✅ CORRECT - data-last
option.Map(transformFunc)(myOption)

// ✅ CORRECT - in pipeline
F.Pipe2(
    myOption,
    O.Map(transformFunc),
    O.GetOrElse(LZ.Of("default")),
)
```

**Severity**: High — breaks composition

### 3. Point-Free Style

**Rule**: Prefer composing named functions with `Flow` and `Pipe` over inline anonymous functions.

**Check for**:
```go
// ❌ AVOID - unnecessary lambda wrapping
pipeline := F.Flow2(
    func(s string) O.Option[string] { return O.FromPredicate(S.IsNonEmpty)(s) },
    func(o O.Option[string]) string { return O.GetOrElse(func() string { return "" })(o) },
)

// ✅ CORRECT - point-free composition
pipeline := F.Flow2(
    O.FromPredicate(S.IsNonEmpty),   // string -> Option[string]
    O.GetOrElse(LZ.Of("")),          // Option[string] -> string; LZ = v2/lazy
)

// ❌ AVOID - inline comparison
A.Filter(func(x int) bool { return x > 18 })

// ✅ CORRECT - use numeric combinator
A.Filter(N.MoreThan(18))

// ❌ AVOID - field-access lambda inside a pipeline step
RIO.Map(func(u User) string { return u.Name })

// ✅ CORRECT - named leaf accessor or lens getter
RIO.Map(nameLens.Get)

// ❌ AVOID - lambda that only threads its argument into a pipeline
RIO.Chain(func(u User) RIO.ReaderIOResult[[]Order] {
    return F.Pipe1(fetchOrders(u.ID), RIO.LogEntryExit[[]Order]("fetchOrders"))
})

// ✅ CORRECT - compose the steps
RIO.Chain(F.Flow3(getUserID, fetchOrders, RIO.LogEntryExit[[]Order]("fetchOrders")))

// ❌ AVOID - hand-written Reader/IO closures around a Go function
func fetchUser(id int) RIO.ReaderIOResult[User] {
    return func(ctx context.Context) func() R.Result[User] {
        return func() R.Result[User] { return R.TryCatchError(repo.FindUser(ctx, id)) }
    }
}

// ✅ CORRECT - lift it
fetchUser := RIO.Eitherize1(repo.FindUser) // repo.FindUser: func(context.Context, int) (User, error)
```

Lambdas are acceptable only at the **leaves**: a struct field accessor (prefer a
generated lens), a setter passed to `L.MakeLens`, a multi-field formatter, a
side-effecting sink at the program edge (e.g. an HTTP response writer), or a
blocking leaf that must `select` on `ctx.Done()`. Everything composed on top of
the leaves should be point-free.

**Severity**: Medium — impacts readability and maintainability

### 4. Prefer Result over Either

**Rule**: Use `Result[A]` (which is `Either[error, A]`) when the error type is Go's `error`. Reserve `Either` for custom error types.

**Check for**:
```go
// ❌ AVOID - Either with error
func fetchData() E.Either[error, Data] { ... }

// ✅ CORRECT - use Result
func fetchData() R.Result[Data] { ... }

// ✅ CORRECT - Either with custom error type
func validate() E.Either[ValidationError, Data] { ... }
```

**Severity**: Medium — Result is more idiomatic for Go errors

### 5. IO Laziness

**Rule**: IO values are lazy (`IO[A]` is `func() A`). They must be called with `()` to execute.

**Check for**:
```go
// ❌ WRONG - forgot to execute
result := readConfig("config.json")  // returns IO[Config], not Config

// ✅ CORRECT - execute with ()
result := readConfig("config.json")()

// ❌ WRONG - in ReaderIOResult, forgot inner ()
res := pipeline(ctx)         // returns func() Result[A], nothing has run yet

// ✅ CORRECT - execute both context and IO
res := pipeline(ctx)()       // Result[A] — ONE value

// ❌ WRONG - Result[A] is a single value, not a (value, error) tuple
value, err := pipeline(ctx)()

// ✅ CORRECT - unwrap to idiomatic Go at the boundary
value, err := R.Unwrap(pipeline(ctx)())
```

**Severity**: Critical — code won't execute

### 6. Monad Selection

**Rule**: Use the simplest monad that covers your needs. Escalate only when necessary.

**Check for**:
```go
// ❌ AVOID - using ReaderIOResult for pure computation
// (also note: in context/readerioresult the context is baked in —
//  it is RIO.Of[A] and RIO.Map[A, B], with no environment type parameter)
func processUsers(users []User) RIO.ReaderIOResult[string] {
    return F.Pipe1(
        RIO.Of(users),
        RIO.Map(pureTransform),
    )
}

// ✅ CORRECT - pure computation, no monad needed
func processUsers() func([]User) string {
    return F.Flow2(
        A.FilterMap(toAdultName()),
        A.Intercalate(S.Monoid)(","),
    )
}
```

**Severity**: Medium — unnecessary complexity

**Escalation path**: `Option` → `Result` → `IOResult` → `ReaderIOResult` → `Effect`

### 7. Effect vs ReaderIOResult

**Rule**: Use `Effect[C, A]` for services with typed dependencies. Use `ReaderIOResult` only when you truly only need `context.Context`.

**Check for**:
```go
// ❌ AVOID - stuffing deps into context.Context
func fetchUser(id int) RIO.ReaderIOResult[User] {
    return func(ctx context.Context) func() R.Result[User] {
        db := ctx.Value("db").(DBClient)  // runtime type assertion
        // ...
    }
}

// ✅ CORRECT - typed dependencies with Effect
type Deps struct {
    DB     DBClient
    Logger Logger
}

// Lift an idiomatic function that receives the deps and the context.
// queryUser is func(Deps, context.Context, int) (User, error); deps.DB is compile-time checked.
func fetchUser() EF.Kleisli[Deps, int, User] {
    return EF.Eitherize1(queryUser)
}
```

`Effect[Deps, User]` IS `func(Deps) ReaderIOResult[User]`. `EF.Asks` is only for pure
projections `func(Deps) A`; feeding it a function that returns a `ReaderIOResult`
silently produces the nested `Effect[Deps, ReaderIOResult[User]]` — flag that as High.

Also flag `EF.Map(f)` and `EF.Provide(deps)(eff)` without annotations — `Map[C, A, B]` usually
cannot infer `C`, and `Provide[A, C]` cannot infer `A` through the function it returns. Write
`EF.Map[Deps](f)` and `EF.Provide[string](deps)`.

Also flag request-scoped data (request IDs, principal, deadlines) placed in `C`, one wide dependency type used by every function instead of narrow `XxxDeps` widened with `EF.Local`, and `Provide` / `RunSync` inside library code. See the `fp-go-effect` skill.

**Severity**: High — type safety and testability

### 8. Lifting Go Functions

**Rule**: Use `Eitherize1`..`EitherizeN` to lift Go functions returning `(T, error)` into Result.

**Check for**:
```go
// ❌ AVOID - manual error handling
func parseNumber(s string) R.Result[int] {
    n, err := strconv.Atoi(s)
    if err != nil {
        return R.Left[int](err)   // Left[A] — A is the success type
    }
    return R.Of(n)                // NOT R.Right[error](n); Right[A any](v A)
}

// ✅ CORRECT - use Eitherize
var parseNumber = R.Eitherize1(strconv.Atoi)

// ✅ CORRECT - in pipeline
pipeline := F.Flow2(
    R.Eitherize1(strconv.Atoi),
    R.Map(N.Mul(2)),
)
```

**Severity**: Medium — reduces boilerplate

### 9. Do-Notation with Lenses

**Rule**: Use lenses with `Bind`/`ApS` instead of manual setter functions.

**Check for**:
```go
// ❌ AVOID - manual setter functions
func setUser(u User) func(State) State {
    return func(s State) State { s.User = u; return s }
}

pipeline := F.Pipe1(
    RIO.Do(State{}),
    RIO.Bind(setUser, fetchUser),
)

// ✅ CORRECT - use lens
var userLens = L.MakeLens(
    func(s State) User { return s.User },
    func(s State, u User) State { s.User = u; return s },
)

pipeline := F.Pipe1(
    RIO.Do(State{}),
    RIO.Bind(userLens.Set, fetchUser),
)

// ✅ EVEN BETTER - use code generation
//go:generate go run github.com/IBM/fp-go/v2 lens --dir . --filename gen_lens.go

// fp-go:Lens
type State struct {
    User User
}

// Then use generated lens
lenses := MakeStateLenses()
pipeline := F.Pipe1(
    RIO.Do(State{}),
    RIO.Bind(lenses.User.Set, fetchUser),
)
```

**Severity**: Medium — maintainability and consistency

### 10. Bind vs ApS

**Rule**: Use `Bind` when the step depends on accumulated state; use `ApS` when steps are independent.

**Check for**:
```go
// ❌ WRONG - using Bind when steps are independent
pipeline := F.Pipe2(
    RIO.Do(Summary{}),
    RIO.Bind(userLens.Set, func(_ Summary) RIO.ReaderIOResult[User] {
        return fetchUser(42)  // doesn't use state
    }),
    RIO.Bind(weatherLens.Set, func(_ Summary) RIO.ReaderIOResult[Weather] {
        return fetchWeather("NYC")  // doesn't use state
    }),
)

// ✅ CORRECT - use ApS for independent steps
pipeline := F.Pipe2(
    RIO.Do(Summary{}),
    RIO.ApS(userLens.Set, fetchUser(42)),
    RIO.ApS(weatherLens.Set, fetchWeather("NYC")),
)

// ✅ CORRECT - ApS for the independent first step, Bind for the dependent one
pipeline := F.Pipe2(
    RIO.Do(Pipeline{}),
    RIO.ApS(userLens.Set, fetchUser(42)),
    RIO.Bind(configLens.Set, F.Flow2(userLens.Get, fetchConfigForUser)),
)
```

**Severity**: Medium — semantic clarity

### 11. TraverseArray Usage

**Rule**: Use `TraverseArray` to process slices monadically, not manual loops with error accumulation.

**Check for**:
```go
// ❌ AVOID - manual loop with error handling
func fetchAll(ids []int) RIO.ReaderIOResult[[]User] {
    return func(ctx context.Context) func() R.Result[[]User] {
        return func() R.Result[[]User] {
            users := make([]User, 0, len(ids))
            for _, id := range ids {
                user, err := R.Unwrap(fetchUser(id)(ctx)())
                if err != nil {
                    return R.Left[[]User](err)
                }
                users = append(users, user)
            }
            return R.Of(users)
        }
    }
}

// ✅ CORRECT - use TraverseArray, point-free (return the Kleisli, don't take ids)
func fetchAll() RIO.Kleisli[[]int, []User] {
    return RIO.TraverseArray(fetchUser)
}
```

**Severity**: High — idiomatic functional pattern

### 12. Logging Side Effects

**Rule**: Log with the `Tap*` operators, which run the side effect and pass the original value (or error) through unchanged. Prefer structured `TapSLog`; use `TapIOK(IO.Logf…)` for printf-style logs and `LogEntryExit` for entry/exit logs. See the `fp-go-logging` skill.

**Check for**:
```go
// ❌ AVOID - breaking the pipeline for logging
pipeline := F.Pipe1(
    fetchUser(42),
    RIO.Chain(func(user User) RIO.ReaderIOResult[User] {
        log.Printf("Fetched user: %v", user)
        return RIO.Of(user)
    }),
)

// ✅ CORRECT - structured logging with TapSLog (logs value or error)
pipeline := F.Pipe1(
    fetchUser(42),
    RIO.TapSLog[User]("User fetched"),
)

// ✅ CORRECT - printf-style logging with TapIOK
pipeline := F.Pipe1(
    fetchUser(42),
    RIO.TapIOK(IO.Logf[User]("Fetched user: %v")),
)
```

Also flag `ChainFirstIOK` used for logging (Low: works, but `TapIOK` states the intent) and `slog.Info` inside `Map` (Medium: a side effect in a pure function that also bypasses the context logger).

**Severity**: Low — code quality

### 13. Prefer Functions over Variables

**Rule**: Wrap pipeline results in functions, not package-level vars.

**Check for**:
```go
// ❌ WRONG - var is allocated even if never called
var processUser = F.Flow2(getName, strings.ToUpper)

// ✅ CORRECT - zero cost until called
func processUser() func(User) string {
    return F.Flow2(getName, strings.ToUpper)
}
```

The rule targets composed pipelines (`Pipe`/`Flow` results). A `var` is fine for lenses and for a single pre-bound helper such as `var parseNumber = R.Eitherize1(strconv.Atoi)` or `var getHost = hostLens.Get` (same rule as the `fp-go-pipe-flow` skill).

**Severity**: Low — performance and dead code elimination

### 14. Type Parameter Order

**Rule**: Non-inferrable type parameters come first, so an explicit annotation only ever needs the leading prefix.

Which params are non-inferrable differs per package — check the signature rather than assuming:

```go
// option / result / ioresult: Map[A, B](f func(A) B) — BOTH inferable from f
O.Map(toLength)              // ✅ preferred, no annotation at all
O.Map[string, int](toLength) // ✅ legal but redundant (note the order: A then B)

// Ap[B, A](fa M[A]) — B is not recoverable from fa, so it leads
O.Ap[int](fa)                // ✅

// either / reader / readerio*: the error or environment type leads
E.Map[error](f)              // ✅ either.Map[E, A, B]
RD.Map[context.Context](f)   // ✅ reader.Map[R, A, B]

// effect: C leads and is often not inferable
EF.Map[Deps](f)              // ✅
EF.Provide[string](deps)     // ✅ Provide[A, C] cannot infer A through its result
```

Flag an annotation that is in the wrong order (it will not compile) or one that
restates what the compiler already infers.

**Severity**: Low — compilation errors or verbosity

### 15. Lens Composition

**Rule**: Use `Compose`/`ComposeRef` for nested struct access, not manual chaining.

**Check for**:
```go
// ❌ AVOID - manual nested access
func getStreetName(p Person) string {
    if p.Address != nil && p.Address.Street != nil {
        return p.Address.Street.Name
    }
    return ""
}

// ✅ CORRECT - compose lenses
streetNameInPerson := F.Pipe2(
    personAddressLens,
    LO.Compose[Person, *Street](defaultAddress)(addressStreetLens),
    LO.ComposeOption[Person, string](defaultStreet)(streetNameLens),
)

name := streetNameInPerson.Get(person)  // Option[string]
```

**Severity**: Medium — immutability and composability

### 16. Immutability / No Hidden Mutation

**Rule**: Functions passed to `Map`, `Chain`, `Filter`, etc. must be pure — they must not mutate variables captured from an outer scope, and lens setters must not mutate shared slice/map fields in place.

**Check for**:
```go
// ❌ WRONG - closure mutates a captured slice
var acc []string
A.Map(func(u User) User {
    acc = append(acc, u.Name)  // hidden side effect
    return u
})

// ✅ CORRECT - derive a new value, no captured mutation
names := F.Pipe1(users, A.Map(getName))

// ❌ WRONG - lens setter mutates a shared slice in place
//   append may reuse the original backing array (shallow struct copy)
func(u User, t []string) User { u.Tags = append(u.Tags, t...); return u }

// ✅ CORRECT - assign a freshly built value
func(u User, t []string) User { u.Tags = t; return u }
```

**Severity**: High — a mutating closure silently defeats fp-go's guarantees and breaks under `TraverseArray`/concurrency.

### 17. Context Access and Scoping

**Rule**: Read `context.Context` values with `AskValue`, and scope values, timeouts and deadlines with the `WithValue` / `WithTimeout` / `WithDeadline` operators (or `Local`). Do not type-assert `ctx.Value` or derive contexts by hand inside pipelines. The operators exist in `context/readerio`, `context/readerresult`, `context/readerioresult`, `context/statereaderioresult` and `idiomatic/context/readerresult`.

**Check for**:
```go
// ❌ WRONG - panics if the key is missing or has another type; plain string key
getUser := RIO.FromReader(func(ctx context.Context) string {
    return ctx.Value("user").(string)
})

// ✅ CORRECT - typed key, Option result, caller decides what "missing" means
type ctxKey string
const userKey ctxKey = "user"
getUser := F.Pipe1(RIO.AskValue[string](userKey), RIO.Map(O.GetOrElse(LZ.Of("anonymous"))))

// ❌ WRONG - hand-derived context; cancel discarded -> leaked timer
RIO.Local[A](func(ctx context.Context) ContextCancel {
    tctx, _ := context.WithTimeout(ctx, 5*time.Second)
    return pair.MakePair(func() {}, tctx)
})

// ❌ WRONG - scoping done outside the pipeline, by hand
ctx, cancel := context.WithTimeout(context.WithValue(ctx, userKey, u), 5*time.Second)
defer cancel()
res := pipeline(ctx)()

// ✅ CORRECT - scoping as operators; cancel always released
res := F.Pipe2(
    pipeline,
    RIO.WithTimeout[A](5*time.Second),
    RIO.WithValue[A](userKey, u),
)(ctx)()

// ❌ WRONG - Unpack + defer just to install a logger
cancel, lctx := pair.Unpack(logging.WithLogger(l)(ctx)); defer cancel()

// ✅ CORRECT - WithLogger already has Local's shape
F.Pipe1(pipeline, RIO.Local[A](logging.WithLogger(l)))
```

Also flag:
- string or other exported key types (`"user"`, `int`) — use an unexported `type ctxKey string`
- dependencies (DB, config, clients) stored in the context — see §7, use `Effect`
- outside pipelines, `context.WithValue(ctx, k, v)` where `CR.WithValue[V](k)(v)(ctx)` from `context/reader` would keep code consistent (Low)

**Severity**: High for panicking assertions and leaked cancel functions; Medium for hand-rolled scoping that has an operator equivalent.

## Review Process

### Step 1: Obtain Git Diff

Get the changes on the PR branch relative to main:

```bash
git diff main...HEAD
```

To list only changed file paths:

```bash
git diff --name-only main...HEAD
```

For a GitHub PR, fetch it first:

```bash
gh pr checkout <PR-number>
git diff main...HEAD
```

### Step 2: Analyze Changes

First, confirm the branch compiles: run `go build ./...` and `go vet ./...` on the
checked-out branch. Report any build or vet failure as a **Critical** finding —
there is no point reviewing composition style on code that does not compile, and
most fp-go-specific mistakes (wrong leading type parameter, data-first vs
data-last argument order, missing trailing `()`) surface here.

Then, for each modified file:
1. Check import paths (v2 requirement)
2. Validate data-last usage
3. Check for point-free style opportunities
4. Verify monad selection appropriateness
5. Check IO execution (trailing `()`)
6. Validate error handling patterns
7. Check lens usage in do-notation
8. Verify Bind vs ApS usage
9. Look for TraverseArray opportunities
10. Check logging patterns

### Step 3: Submit Findings

Post a review comment on the GitHub PR:

```bash
gh pr review <PR-number> --comment -b "$(cat <<'EOF'
## fp-go Review

**Overall**: Needs Changes

### Critical
- ❌ ...

### High
- ⚠️ ...

### Recommendations
1. ...
EOF
)"
```

For inline annotations on specific lines, use:

```bash
gh api repos/{owner}/{repo}/pulls/<PR-number>/comments \
  -f body="Replace inline lambda with point-free: \`F.Flow2(O.FromPredicate(S.IsNonEmpty), O.GetOrElse(LZ.Of(\"\")))\`" \
  -f commit_id="$(git rev-parse HEAD)" \
  -f path="src/user/handler.go" \
  -F line=42 \
  -f side=RIGHT
```

Alternatively, use the `/code-review --comment` skill to post inline PR annotations automatically.

## Common Issue Categories

| Category | Type | Example |
|----------|------|---------|
| maintainability | dry-principle-violation | Inline lambdas instead of point-free |
| maintainability | naming-intent-review | Non-descriptive variable names |
| functionality | error-handling-review | Missing error propagation |
| functionality | context-handling | `ctx.Value(k).(T)` assertion or discarded `cancel` instead of `AskValue` / `WithTimeout` |
| performance | inefficient-algorithm | Manual loops instead of TraverseArray |
| style | style-consistency-check | Inconsistent import aliases |
| security | sensitive-data-logging | Logging sensitive information |

## Severity Guidelines

- **Critical**: Code won't compile or execute (wrong import path, missing `()`)
- **High**: Type safety issues, incorrect monad usage, breaks composition
- **Medium**: Readability, maintainability, non-idiomatic patterns
- **Low**: Style preferences, minor optimizations

## Example Review Comments

### Import Path Issue

> **Severity**: Critical
> **Issue**: Using v1 import path
>
> The import `github.com/IBM/fp-go/option` is the v1 path. All imports must use v2:
> `github.com/IBM/fp-go/v2/option`
>
> v1 and v2 are incompatible. This will cause compilation errors or runtime issues.

### Point-Free Style

> **Severity**: Medium
> **Issue**: Unnecessary lambda wrapping
>
> This inline lambda can be replaced with point-free composition:
>
> Current:
> ```go
> option.Filter(func(s string) bool { return s != "" })
> ```
>
> Suggested:
> ```go
> option.Filter(S.IsNonEmpty)
> ```
>
> Point-free style is more readable and idiomatic in fp-go.

### Monad Selection

> **Severity**: Medium
> **Issue**: Unnecessary monad for pure computation
>
> This function uses `ReaderIOResult` but performs only pure transformations without IO or context:
>
> ```go
> func processUsers(users []User) RIO.ReaderIOResult[string] {
>     return F.Pipe1(
>         RIO.Of(users),
>         RIO.Map(pureTransform),
>     )
> }
> ```
>
> Suggested:
> ```go
> func processUsers() func([]User) string {
>     return F.Flow2(
>         A.FilterMap(toAdultName()),
>         A.Intercalate(S.Monoid)(","),
>     )
> }
> ```
>
> Use the simplest abstraction that covers your needs.

## Integration with Other Skills

This skill can reference and include:
- `fp-go` — Core fp-go patterns and best practices
- `fp-go-pipe-flow` — Pipe/Flow composition patterns
- `fp-go-http` — HTTP request patterns
- `fp-go-logging` — Logging patterns
- `fp-go-lens` — Lens and optics patterns
- `fp-go-context` — context.Context handling: reading values, scoping, timeouts, cancellation (see §17)
- `fp-go-pattern-matching` — replacing switch / if-else chains with point-free case lists
- `fp-go-effect` — `Effect[C, A]` with typed dependencies in `C`: capability interfaces, `Local`, testing with fakes (see §7)

## Automated Checks

When reviewing, automatically check for:

1. ✅ All imports use `v2` path
2. ✅ No data-first function calls
3. ✅ IO values are executed with `()`
4. ✅ `Result` used instead of `Either[error, A]`
5. ✅ Point-free style: no lambdas above the leaves; `Eitherize` instead of hand-written closures; `ApS` instead of state-ignoring `Bind`
6. ✅ Appropriate monad selection
7. ✅ Lenses used in do-notation
8. ✅ `Bind` vs `ApS` used correctly
9. ✅ `TraverseArray` for slice processing
10. ✅ Logging via `TapSLog` / `TapIOK` / `LogEntryExit` (see the `fp-go-logging` skill)
11. ✅ No hidden mutation in `Map`/`Chain` closures or lens setters
12. ✅ Context values read with `AskValue`; values/timeouts scoped with `WithValue`/`WithTimeout`/`WithDeadline`/`Local` (no `ctx.Value(k).(T)`, no discarded cancel funcs)
13. ✅ Branch compiles (`go build ./...`) and passes `go vet ./...`
14. ✅ Import aliases follow the canonical table (see the `fp-go` skill)

## Output Format

Provide a summary with:
1. **Overall Assessment**: Pass/Needs Changes/Blocked
2. **Critical Issues**: Count and list
3. **High Priority Issues**: Count and list
4. **Medium Priority Issues**: Count and list
5. **Low Priority Issues**: Count and list
6. **Positive Observations**: What was done well
7. **Recommendations**: Suggested improvements

## Example Summary

```markdown
## PR Review Summary

**Overall Assessment**: Needs Changes

### Critical Issues (2)
- ❌ Using v1 import path in `user/handler.go:5`
- ❌ Missing IO execution in `config/loader.go:42`

### High Priority Issues (1)
- ⚠️ `ctx.Value("user").(string)` type assertion instead of `AskValue` in `api/auth.go:31`

### Medium Priority Issues (4)
- 💡 Manual error handling instead of Eitherize in `api/client.go:78`
- 💡 Inline lambda instead of point-free in `user/service.go:23`
- 💡 Using ReaderIOResult for pure computation in `utils/format.go:15`
- 💡 Manual setter instead of lens in `state/pipeline.go:56`

### Low Priority Issues (1)
- 📝 Inconsistent import alias in `handler/http.go:8`

### Positive Observations
- ✅ Excellent use of TraverseArray for parallel requests
- ✅ Proper Effect usage with typed dependencies
- ✅ Good lens composition for nested struct access

### Recommendations
1. Update all imports to v2 path
2. Add trailing `()` to execute IO values
3. Consider using `R.Eitherize1` for Go function lifting
4. Refactor pure computations to use Flow instead of ReaderIOResult
```

## References

- [fp-go v2 Documentation](https://pkg.go.dev/github.com/IBM/fp-go/v2)
- [fp-go GitHub Repository](https://github.com/IBM/fp-go)
- [Functional Programming in Go](https://github.com/IBM/fp-go/blob/main/README.md)